You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/29 10:21:01 UTC
(datafusion) branch gh-readonly-queue/main/pr-24700-d109f1b133f7d90f4806bc89b8945df0b6e029ae created (now 15c097f6c9)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-24700-d109f1b133f7d90f4806bc89b8945df0b6e029ae
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 15c097f6c9 refactor(proto): destructure CASE and IN-list expression serde hooks (#24700)
This branch includes the following new commits:
new 15c097f6c9 refactor(proto): destructure CASE and IN-list expression serde hooks (#24700)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: refactor(proto): destructure CASE and IN-list expression serde hooks (#24700)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24700-d109f1b133f7d90f4806bc89b8945df0b6e029ae
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 15c097f6c9d8887c8f10eb3765942e641aa77a66
Author: Burak Şen <bu...@gmail.com>
AuthorDate: Sat Aug 29 10:20:37 2026 +0000
refactor(proto): destructure CASE and IN-list expression serde hooks (#24700)
## Which issue does this PR close?
- Closes #24616.
## Rationale for this change
Proto hooks that access fields individually can silently omit newly
added state. Exhaustive destructuring makes such omissions compile
errors.
## What changes are included in this PR?
- Exhaustively destructure `CaseExpr`, `CaseBody`, and `InListExpr` in
their encoders.
- Exhaustively destructure their protobuf nodes in the decoders.
- Document `eval_method` and `static_filter` as constructor-derived
state.
The behavior and wire format are unchanged.
## Are these changes tested?
Covered by the existing CASE and IN-list proto hook tests:
```bash
cargo test -p datafusion-physical-expr --features proto case::proto_tests
cargo test -p datafusion-physical-expr --features proto in_list::proto_tests
```
## Are there any user-facing changes?
No.
---
datafusion/physical-expr/src/expressions/case.rs | 64 +++++++++++++++++-----
.../physical-expr/src/expressions/in_list.rs | 44 +++++++++++----
2 files changed, 83 insertions(+), 25 deletions(-)
diff --git a/datafusion/physical-expr/src/expressions/case.rs b/datafusion/physical-expr/src/expressions/case.rs
index ae0812e0fd..e95e9e570c 100644
--- a/datafusion/physical-expr/src/expressions/case.rs
+++ b/datafusion/physical-expr/src/expressions/case.rs
@@ -1419,16 +1419,26 @@ impl PhysicalExpr for CaseExpr {
) -> Result<Option<datafusion_proto_models::protobuf::PhysicalExprNode>> {
use datafusion_proto_models::protobuf;
+ let Self {
+ body,
+ // Derived from `body` by `try_new` on decode.
+ eval_method: _,
+ } = self;
+ let CaseBody {
+ expr,
+ when_then_expr,
+ else_expr,
+ } = body;
+
Ok(Some(protobuf::PhysicalExprNode {
expr_id: None,
expr_type: Some(protobuf::physical_expr_node::ExprType::Case(Box::new(
protobuf::PhysicalCaseNode {
- expr: self
- .expr()
+ expr: expr
+ .as_ref()
.map(|expr| ctx.encode_child(expr).map(Box::new))
.transpose()?,
- when_then_expr: self
- .when_then_expr()
+ when_then_expr: when_then_expr
.iter()
.map(|(when_expr, then_expr)| {
Ok(protobuf::PhysicalWhenThen {
@@ -1437,8 +1447,8 @@ impl PhysicalExpr for CaseExpr {
})
})
.collect::<Result<Vec<_>>>()?,
- else_expr: self
- .else_expr()
+ else_expr: else_expr
+ .as_ref()
.map(|expr| ctx.encode_child(expr).map(Box::new))
.transpose()?,
},
@@ -1462,30 +1472,36 @@ impl CaseExpr {
protobuf::physical_expr_node::ExprType::Case,
"CaseExpr",
);
+ let protobuf::PhysicalCaseNode {
+ expr,
+ when_then_expr,
+ else_expr,
+ } = &**case;
Ok(Arc::new(CaseExpr::try_new(
- case.expr
- .as_deref()
- .map(|expr| ctx.decode(expr))
- .transpose()?,
- case.when_then_expr
+ expr.as_deref().map(|expr| ctx.decode(expr)).transpose()?,
+ when_then_expr
.iter()
.map(|when_then| {
+ let protobuf::PhysicalWhenThen {
+ when_expr,
+ then_expr,
+ } = when_then;
Ok((
ctx.decode_required_expression(
- when_then.when_expr.as_ref(),
+ when_expr.as_ref(),
"CaseExpr",
"when_expr",
)?,
ctx.decode_required_expression(
- when_then.then_expr.as_ref(),
+ then_expr.as_ref(),
"CaseExpr",
"then_expr",
)?,
))
})
.collect::<Result<Vec<_>>>()?,
- case.else_expr
+ else_expr
.as_deref()
.map(|expr| ctx.decode(expr))
.transpose()?,
@@ -3419,6 +3435,26 @@ mod proto_tests {
assert!(case_node.else_expr.is_some());
}
+ #[test]
+ fn try_to_proto_distinguishes_optional_case_exprs() {
+ let encoder = StubEncoder::ok();
+ let ctx = PhysicalExprEncodeCtx::new(&encoder);
+
+ for (expr, else_expr) in [(None, Some(lit(0_i32))), (Some(lit(true)), None)] {
+ let case = CaseExpr::try_new(expr, vec![(lit(true), lit(1_i32))], else_expr)
+ .unwrap();
+ let node = case.try_to_proto(&ctx).unwrap().unwrap();
+ let Some(protobuf::physical_expr_node::ExprType::Case(case_node)) =
+ node.expr_type
+ else {
+ panic!("expected a CaseExpr node");
+ };
+
+ assert_eq!(case_node.expr.is_some(), case.expr().is_some());
+ assert_eq!(case_node.else_expr.is_some(), case.else_expr().is_some());
+ }
+ }
+
#[test]
fn try_to_proto_propagates_child_encode_error() {
let case = proto_case_fixture();
diff --git a/datafusion/physical-expr/src/expressions/in_list.rs b/datafusion/physical-expr/src/expressions/in_list.rs
index 3c260190b8..a505f3d077 100644
--- a/datafusion/physical-expr/src/expressions/in_list.rs
+++ b/datafusion/physical-expr/src/expressions/in_list.rs
@@ -264,15 +264,20 @@ impl InListExpr {
protobuf::physical_expr_node::ExprType::InList,
"InList",
);
+ let protobuf::PhysicalInListNode {
+ expr,
+ list,
+ negated,
+ } = &**node;
let expr =
- ctx.decode_required_expression(node.expr.as_deref(), "InListExpr", "expr")?;
- let list = ctx.decode_children_expressions(&node.list)?;
+ ctx.decode_required_expression(expr.as_deref(), "InListExpr", "expr")?;
+ let list = ctx.decode_children_expressions(list)?;
Ok(Arc::new(InListExpr::try_new(
expr,
list,
- node.negated,
+ *negated,
ctx.schema(),
)?))
}
@@ -480,13 +485,21 @@ impl PhysicalExpr for InListExpr {
) -> Result<Option<datafusion_proto_models::protobuf::PhysicalExprNode>> {
use datafusion_proto_models::protobuf;
+ let Self {
+ expr,
+ list,
+ negated,
+ // Lookup set rebuilt from `list` by `try_new` on decode.
+ static_filter: _,
+ } = self;
+
Ok(Some(protobuf::PhysicalExprNode {
expr_id: None,
expr_type: Some(protobuf::physical_expr_node::ExprType::InList(Box::new(
protobuf::PhysicalInListNode {
- expr: Some(Box::new(ctx.encode_child(&self.expr)?)),
- list: ctx.encode_children_expressions(&self.list)?,
- negated: self.negated,
+ expr: Some(Box::new(ctx.encode_child(expr)?)),
+ list: ctx.encode_children_expressions(list)?,
+ negated: *negated,
},
))),
}))
@@ -3961,15 +3974,15 @@ mod proto_tests {
}
/// An `InListExpr` over a column with one literal value.
- fn in_list_fixture() -> InListExpr {
+ fn in_list_fixture(negated: bool) -> InListExpr {
let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
- InListExpr::try_new(col("a", &schema).unwrap(), vec![lit(1)], false, &schema)
+ InListExpr::try_new(col("a", &schema).unwrap(), vec![lit(1)], negated, &schema)
.unwrap()
}
#[test]
fn try_to_proto_encodes_in_list() {
- let in_list = in_list_fixture();
+ let in_list = in_list_fixture(false);
let encoder = StubEncoder::ok();
let ctx = PhysicalExprEncodeCtx::new(&encoder);
@@ -3987,11 +4000,20 @@ mod proto_tests {
assert!(!in_list_node.negated);
assert!(in_list_node.expr.is_some());
assert_eq!(in_list_node.list.len(), 1);
+
+ assert!(matches!(
+ in_list_fixture(true)
+ .try_to_proto(&ctx)
+ .unwrap()
+ .unwrap()
+ .expr_type,
+ Some(physical_expr_node::ExprType::InList(node)) if node.negated
+ ));
}
#[test]
fn try_to_proto_propagates_expr_encode_error() {
- let in_list = in_list_fixture();
+ let in_list = in_list_fixture(false);
let encoder = StubEncoder::failing_on(1);
let ctx = PhysicalExprEncodeCtx::new(&encoder);
let err = in_list.try_to_proto(&ctx).unwrap_err();
@@ -4000,7 +4022,7 @@ mod proto_tests {
#[test]
fn try_to_proto_propagates_list_encode_error() {
- let in_list = in_list_fixture();
+ let in_list = in_list_fixture(false);
// Call 1 is for `expr`, Call 2 is for the first element of `list`
let encoder = StubEncoder::failing_on(2);
let ctx = PhysicalExprEncodeCtx::new(&encoder);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org