Skip to content

refactor: Port IsNotNullExpr proto serialization hooks#22532

Open
chakkk309 wants to merge 1 commit into
apache:mainfrom
chakkk309:fix-is-not-null-proto-hooks
Open

refactor: Port IsNotNullExpr proto serialization hooks#22532
chakkk309 wants to merge 1 commit into
apache:mainfrom
chakkk309:fix-is-not-null-proto-hooks

Conversation

@chakkk309
Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

This is part of #22418, which migrates built-in PhysicalExpr implementations away from the central protobuf serialization / deserialization chains.

IsNotNullExpr can now own its protobuf serialization through PhysicalExpr::try_to_proto and its deserialization through IsNotNullExpr::try_from_proto, matching the pattern introduced for Column and BinaryExpr.

What changes are included in this PR?

  • Adds PhysicalExpr::try_to_proto support for IsNotNullExpr.
  • Adds IsNotNullExpr::try_from_proto.
  • Wires IsNotNullExpr deserialization in from_proto.rs through the new hook.
  • Removes the old IsNotNullExpr serialization branch from the central to_proto.rs downcast chain.
  • Adds direct proto hook tests for:
    • successful IsNotNullExpr roundtrip
    • wrong protobuf expression variant
    • missing required child expression

Are these changes tested?

Yes.

cargo fmt --all --check
git diff --check
cargo test -p datafusion-physical-expr --features proto is_not_null_
cargo clippy -p datafusion-proto --tests -- -D warnings
cargo test -p datafusion-proto --test proto_integration 

Are there any user-facing changes?

No.

@chakkk309 chakkk309 changed the title Port IsNotNullExpr proto serialization hooks refactor: Port IsNotNullExpr proto serialization hooks May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port IsNotNullExpr to use try_to_proto / try_from_proto

1 participant