Skip to content

Commit bc9f439

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` also standardizes the "Missing required field" error so each expression no longer spells its own message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1b8451c commit bc9f439

1 file changed

Lines changed: 52 additions & 0 deletions

File tree

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

Lines changed: 52 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,40 @@ 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()`). `field` names
662+
/// the proto field for the error message so authors don't each hand-roll
663+
/// a "missing required field" string.
664+
pub fn decode_required_expression(
665+
&self,
666+
node: Option<&PhysicalExprNode>,
667+
field: &str,
668+
) -> Result<Arc<dyn PhysicalExpr>> {
669+
let node = node.ok_or_else(|| {
670+
datafusion_common::internal_datafusion_err!(
671+
"Missing required field {field:?}"
672+
)
673+
})?;
674+
self.decode(node)
675+
}
676+
677+
/// Decode a sequence of child nodes, preserving order.
678+
///
679+
/// Convenience wrapper over [`Self::decode`] for expressions holding a
680+
/// `repeated` proto field (e.g. the `list` of an `InList`). The first
681+
/// decode error short-circuits.
682+
pub fn decode_children_expressions<'b, I>(
683+
&self,
684+
nodes: I,
685+
) -> Result<Vec<Arc<dyn PhysicalExpr>>>
686+
where
687+
I: IntoIterator<Item = &'b PhysicalExprNode>,
688+
{
689+
nodes.into_iter().map(|node| self.decode(node)).collect()
690+
}
639691
}
640692

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

0 commit comments

Comments
 (0)