Sub-issue of #22418. Port LikeExpr to own its protobuf (de)serialization via the try_to_proto / try_from_proto hooks added in #21929, removing it from the central downcast chain.
Reference: copy the Column migration in #21929. Mind the asymmetry — try_to_proto is a trait override, try_from_proto is an inherent fn (see steps).
Where LikeExpr lives
datafusion/physical-expr/src/expressions/like.rs (crate datafusion-physical-expr).
Steps
- Add
try_to_proto inside the impl PhysicalExpr for LikeExpr block — it overrides a trait method, so it must live there, feature-gated #[cfg(feature = "proto")].
⚠️ Do not put it in an inherent impl LikeExpr block. The serializer calls it through &dyn PhysicalExpr, so an inherent method is silently never reached and serialization keeps using the old chain. This is the most common review bounce on these PRs.
- Add
LikeExpr::try_from_proto(node, ctx) as an inherent associated fn (#[cfg(feature = "proto")] impl LikeExpr) and wire it into the matching ExprType::… arm in datafusion/proto/src/physical_plan/from_proto.rs.
- In this same PR, delete the
LikeExpr arm from datafusion/proto/src/physical_plan/to_proto.rs and the old ExprType::… decode arm in from_proto.rs. This is not cleanup-for-later — it is the only thing that proves your hook is actually reached. If the roundtrip tests still pass after deleting the arm, it works; if you leave the arm, the tests pass through dead code.
- Keep the wire format byte-for-byte identical.
expr_id is None here (only DynamicFilterPhysicalExpr sets it).
- Keep
use datafusion_proto_models::protobuf; inside the method bodies (matches the Column reference).
Tests
- Existing
roundtrip_physical_plan / roundtrip_physical_expr must still pass: cargo test -p datafusion-proto --test proto_integration.
- Add a direct hook test in
datafusion/physical-expr/src/expressions/like.rs covering both directions and a bad-input case (wrong expr_type variant / missing child → clean error, not panic). Reviewers have asked for this on every reviewed PR.
- Put
#[cfg(test)] mod tests at the end of the file or clippy -D warnings fails (items-after-test-module).
Before you push
- Fill in every section of the PR template.
cargo fmt --all and cargo clippy --all-targets --all-features -- -D warnings must pass.
Checklist
Sub-issue of #22418. Port
LikeExprto own its protobuf (de)serialization via thetry_to_proto/try_from_protohooks added in #21929, removing it from the central downcast chain.Reference: copy the
Columnmigration in #21929. Mind the asymmetry —try_to_protois a trait override,try_from_protois an inherent fn (see steps).Where
LikeExprlivesdatafusion/physical-expr/src/expressions/like.rs(cratedatafusion-physical-expr).Steps
try_to_protoinside theimpl PhysicalExpr for LikeExprblock — it overrides a trait method, so it must live there, feature-gated#[cfg(feature = "proto")].impl LikeExprblock. The serializer calls it through&dyn PhysicalExpr, so an inherent method is silently never reached and serialization keeps using the old chain. This is the most common review bounce on these PRs.LikeExpr::try_from_proto(node, ctx)as an inherent associated fn (#[cfg(feature = "proto")] impl LikeExpr) and wire it into the matchingExprType::…arm indatafusion/proto/src/physical_plan/from_proto.rs.LikeExprarm fromdatafusion/proto/src/physical_plan/to_proto.rsand the oldExprType::…decode arm infrom_proto.rs. This is not cleanup-for-later — it is the only thing that proves your hook is actually reached. If the roundtrip tests still pass after deleting the arm, it works; if you leave the arm, the tests pass through dead code.expr_idisNonehere (onlyDynamicFilterPhysicalExprsets it).use datafusion_proto_models::protobuf;inside the method bodies (matches theColumnreference).Tests
roundtrip_physical_plan/roundtrip_physical_exprmust still pass:cargo test -p datafusion-proto --test proto_integration.datafusion/physical-expr/src/expressions/like.rscovering both directions and a bad-input case (wrongexpr_typevariant / missing child → clean error, not panic). Reviewers have asked for this on every reviewed PR.#[cfg(test)] mod testsat the end of the file orclippy -D warningsfails (items-after-test-module).Before you push
cargo fmt --allandcargo clippy --all-targets --all-features -- -D warningsmust pass.Checklist
try_to_protoinimpl PhysicalExpr for LikeExpr(not inherent),#[cfg(feature="proto")]LikeExpr::try_from_protowired intofrom_proto.rsto_proto.rs+from_proto.rsarms deleted in this PRfmt+clippy -D warningsclean; PR template filled in