Skip to content

Commit 83eae7c

Browse files
adriangbclaude
andcommitted
feat(physical-expr-common): add list/required expr helpers to proto ctx
Expressions migrating to the `try_to_proto`/`try_from_proto` hooks (see #21835, #22418) repeatedly hand-roll the same boilerplate the central `datafusion-proto` match already factors out via `parse_physical_exprs`, `parse_required_physical_expr`, and `serialize_physical_exprs`. Those free functions can't be reused by expression authors — they take `PhysicalProtoConverterExtension`/`PhysicalPlanDecodeContext`, which the ctx structs deliberately hide. Add the same shapes as thin methods on the ctx structs, built on the existing `encode_child`/`decode` primitives: - `PhysicalExprEncodeCtx::encode_children_expressions` - `PhysicalExprDecodeCtx::decode_required_expression` - `PhysicalExprDecodeCtx::decode_children_expressions` `decode_required_expression` takes the expression name and proto field so its "missing required field" error still names where the error is (e.g. `InListExpr is missing required field 'expr'`), while keeping the format in one place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1b8451c commit 83eae7c

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

datafusion/physical-expr-common/src/physical_expr.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,24 @@ pub mod proto_encode {
556556
) -> Result<PhysicalExprNode> {
557557
self.encoder.encode(expr)
558558
}
559+
560+
/// Encode a sequence of child expressions, preserving order.
561+
///
562+
/// Convenience wrapper over [`Self::encode_child`] for expressions
563+
/// holding a `repeated` proto field (e.g. the `list` of an `InList`).
564+
/// The first encode error short-circuits.
565+
pub fn encode_children_expressions<'b, I>(
566+
&self,
567+
exprs: I,
568+
) -> Result<Vec<PhysicalExprNode>>
569+
where
570+
I: IntoIterator<Item = &'b Arc<dyn PhysicalExpr>>,
571+
{
572+
exprs
573+
.into_iter()
574+
.map(|expr| self.encode_child(expr))
575+
.collect()
576+
}
559577
}
560578

561579
/// Internal dispatch trait. Implementors live in `datafusion-proto` and
@@ -636,6 +654,43 @@ pub mod proto_decode {
636654
pub fn decode(&self, node: &PhysicalExprNode) -> Result<Arc<dyn PhysicalExpr>> {
637655
self.decoder.decode(node, self.schema)
638656
}
657+
658+
/// Decode a required child node, erroring if it is absent.
659+
///
660+
/// Proto child expressions are encoded as `Option<Box<PhysicalExprNode>>`;
661+
/// pass the field directly (e.g. `node.expr.as_deref()`). `expr_name`
662+
/// is the expression being decoded (e.g. `"InListExpr"`) and `field`
663+
/// the proto field (e.g. `"expr"`); both are woven into the error so
664+
/// it names *where* the missing field is, without each author
665+
/// hand-rolling the string.
666+
pub fn decode_required_expression(
667+
&self,
668+
node: Option<&PhysicalExprNode>,
669+
expr_name: &str,
670+
field: &str,
671+
) -> Result<Arc<dyn PhysicalExpr>> {
672+
let node = node.ok_or_else(|| {
673+
datafusion_common::internal_datafusion_err!(
674+
"{expr_name} is missing required field '{field}'"
675+
)
676+
})?;
677+
self.decode(node)
678+
}
679+
680+
/// Decode a sequence of child nodes, preserving order.
681+
///
682+
/// Convenience wrapper over [`Self::decode`] for expressions holding a
683+
/// `repeated` proto field (e.g. the `list` of an `InList`). The first
684+
/// decode error short-circuits.
685+
pub fn decode_children_expressions<'b, I>(
686+
&self,
687+
nodes: I,
688+
) -> Result<Vec<Arc<dyn PhysicalExpr>>>
689+
where
690+
I: IntoIterator<Item = &'b PhysicalExprNode>,
691+
{
692+
nodes.into_iter().map(|node| self.decode(node)).collect()
693+
}
639694
}
640695

641696
/// Internal dispatch trait. Implementors live in `datafusion-proto`.

0 commit comments

Comments
 (0)