You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Destructure proto hooks for leaf and unary physical expressions #24613
Literal::field is not encoded. Literal holds value: ScalarValue and field: FieldRef; the encoder writes only value, and Literal::try_from_proto rebuilds the expression with Literal::new(value). A literal constructed with Literal::new_with_metadata loses its field name and metadata on round-trip. Whether that should be carried on the wire or deliberately dropped is the decision this issue should record.
Why
Serde hooks that read state through getters or self.field make an added field invisible to serialization: nothing breaks at compile time, the field simply stops round-tripping, and Debug-comparing round-trip tests do not notice. HashJoinExec::fetch was lost exactly this way (#24165), and #24609 is a second live instance found by applying the convention to one file.
#24164 established the fix -- exhaustive destructuring in both directions -- and applied it to the join plans. The physical-plan plan nodes are done. physical-expr and the datasource* crates were never converted.
What to do
For each hook in scope:
In try_to_proto, start with an exhaustive let Self {{ .. }} -- no .. rest pattern. Fields that are genuinely not serialized bind to _ with a short comment saying why (derived at construction, runtime state, recomputed on decode, carried by a parent message).
In try_from_proto, destructure the prost-generated node struct the same way, so adding a field to datafusion.proto is a compile error in every decoder.
If the destructure turns up a field that should round-trip but has no wire representation, add it to the message and cover it with a test that fails without the fix.
Definition of done
Every field of the plan/expression struct is either serialized or bound to _ with a reason.
Every field of the prost node is destructured in the decoder.
Any newly serialized field has a round-trip test, verified to fail before the fix.
Activity
physical-expr#24611peterxcli commentedon Aug 24, 2026
take
adriangb commentedon Sep 7, 2026
@peterxcli are you still working on this?
kumarUjjawal commentedon Sep 16, 2026
@peterxcli I have unassigned to free this issue and move the EPIC forward, if you are still interested please let me know.
peterxcli commentedon Sep 16, 2026
Thanks! I was waiting for the other patches in this epic to merge first so the PR backlog wouldn't blow up.
peterxcli commentedon Sep 16, 2026
take
peterxcli commentedon Sep 16, 2026
I will file the patch for this asap
fix: preserve literal metadata and destructure leaf and unary proto h…
fix: preserve literal metadata and destructure leaf and unary proto h…