adriangb opened a new pull request, #22513:
URL: https://github.com/apache/datafusion/pull/22513
## Which issue does this PR close?
- Part of #22418 (decentralizing `datafusion-proto` serialization onto the
expressions themselves; follows #21835, #22471, #22503).
## Rationale for this change
Expressions migrating to the `try_to_proto` / `try_from_proto` hooks keep
hand-rolling the same boilerplate that 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 `PhysicalExprEncodeCtx` /
`PhysicalExprDecodeCtx` surfaces deliberately hide.
This was raised in review on #22503 — rather than re-implement the list maps
and "missing required field" checks in every migrated expression, expose the
same shapes on the ctx structs.
## What changes are included in this PR?
- **`datafusion-physical-expr-common`**: three thin convenience methods,
built on the existing `encode_child` / `decode` primitives:
- `PhysicalExprEncodeCtx::encode_children_expressions`
- `PhysicalExprDecodeCtx::decode_required_expression` (also standardizes
the `Missing required field "<name>"` error so each expression no longer spells
its own)
- `PhysicalExprDecodeCtx::decode_children_expressions`
- **`datafusion-physical-expr`**: adopt them in `InListExpr` and `LikeExpr`,
removing the hand-rolled list maps and per-field `ok_or_else` checks.
### Behavior note
`decode_required_expression` couples the presence check with the decode, so
`LikeExpr` now decodes children left-to-right rather than validating both
required fields up front. The end result is unchanged (a missing required field
still errors), but a present sibling is decoded before a later missing field is
reported. The `try_from_proto_rejects_missing_pattern` unit test is updated to
reflect this.
## Are these changes tested?
Yes — covered by existing tests, no new ones needed:
- The isolated `proto_tests` modules in `in_list.rs` and `like.rs` already
exercise all three helpers (list encode/decode, required decode, and the
missing-field + child-error paths) through the migrated `try_to_proto` /
`try_from_proto`.
- The `datafusion-proto` round-trip integration tests (`roundtrip_inlist`,
`roundtrip_like`, `roundtrip_filter_with_not_and_in_list`,
`test_tpch_part_in_list_query_with_real_parquet_data`, etc.) continue to pass.
- `cargo clippy --all-targets --features proto -D warnings` is clean on the
touched crates; `cargo fmt --all` applied.
## Are there any user-facing changes?
No. The only observable change is the wording of the internal "missing
required field" error for `InList`/`Like` decode failures, which is now
standardized.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
