Conversation
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25382 +/- ##
==========================================
+ Coverage 82.65% 82.67% +0.02%
==========================================
Files 1147 1147
Lines 446087 446205 +118
Branches 446087 446205 +118
==========================================
+ Hits 368710 368899 +189
+ Misses 54990 54857 -133
- Partials 22387 22449 +62 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kosiew
left a comment
There was a problem hiding this comment.
Thanks for working on this. The change preserves literal field metadata across physical expression protobuf and JSON round trips while keeping the existing encoding for literals without metadata. The leaf and unary proto hook destructuring also looks behavior-preserving. I did not find any issues that need changes.
Which issue does this PR close?
Closes #24613. Literal field metadata is lost when physical expressions are serialized.
Rationale for this change
Losing this metadata can strip Arrow extension types and custom metadata from projected columns.
What changes are included in this PR?
Use exhaustive destructuring in all seven leaf and unary expression encoders and decoders. Add a literal protobuf message that carries metadata and regenerate the Rust and JSON bindings.
What is the testing strategy for this PR?
A projection round-trip test covers null and non-null literals through protobuf and JSON and was verified to fail before the fix. Existing tests cover plain literals, and a new test rejects messages missing a value. The extended workspace tests and all-feature Clippy pass.
Are there any user-facing changes?
Literal metadata survives physical plan serialization. Literals with metadata use a new protobuf variant that requires an updated reader.