Skip to content

port NegativeExpr to use the try_to_proto / try_from_proto hooks#22483

Open
kevinhongzl wants to merge 2 commits into
apache:mainfrom
kevinhongzl:port-negative-expr-proto-hooks
Open

port NegativeExpr to use the try_to_proto / try_from_proto hooks#22483
kevinhongzl wants to merge 2 commits into
apache:mainfrom
kevinhongzl:port-negative-expr-proto-hooks

Conversation

@kevinhongzl
Copy link
Copy Markdown

@kevinhongzl kevinhongzl commented May 23, 2026

Which issue does this PR close?

Rationale for this change

This change is part of the per-expression proto hooks migration #22418. I moved the serialization and deserialization of NegativeExpr into its proto hooks, keeping it aligned with the new pattern used by migrated physical expressions and reducing special-case branching in the shared conversion code.

What changes are included in this PR?

  • Added try_to_proto and try_from_proto to NegativeExpr
  • Removed the central NegativeExpr serialization branch
  • Updated physical_plan/from_proto.rs to route physical proto decode through try_from_proto

Are these changes tested?

Yes. This PR is verified by running

  • cargo fmt --all -- --check
  • cargo check -p datafusion-physical-expr --features proto
  • cargo check -p datafusion-proto
  • cargo test -p datafusion-proto --test proto_integration roundtrip_physical_plan
  • cargo test -p datafusion-proto --test proto_integration roundtrip_physical_expr
  • git diff --check

Are there any user-facing changes?

No user-facing changes are intended.

@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates proto Related to proto crate labels May 23, 2026
@adriangb
Copy link
Copy Markdown
Contributor

@kevinhongzl could you rebase and resolve conflicts please?

@kevinhongzl
Copy link
Copy Markdown
Author

@adriangb I've fixed the merge conflicts, and it's now ready to be merged.

@adriangb
Copy link
Copy Markdown
Contributor

Could you add some tests please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates proto Related to proto crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port NegativeExpr to use try_to_proto / try_from_proto

2 participants