Scope
| file |
type |
datasource-parquet/src/source.rs |
ParquetSource |
datasource-parquet/src/sink.rs |
ParquetSink |
4 hooks (2 encoders, 2 decoders).
Field drop found in this group
ParquetSource::metadata_size_hint is not serialized. ParquetScanExecNode carries base_conf, predicate and parquet_options only, and the hint is not part of TableParquetOptions either, so a source configured with the public with_metadata_size_hint loses its footer prefetch size on round-trip.
Several other ParquetSource fields are unmentioned by the encoder and need an explicit decision: parquet_file_reader_factory and encryption_factory cannot be serialized and are rebuilt from the decode context (that is already documented on try_from_proto -- it should become a _ binding with the same note), while reverse_row_groups and sort_order_for_reorder need checking against how they are set.
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
Scope
datasource-parquet/src/source.rsParquetSourcedatasource-parquet/src/sink.rsParquetSink4 hooks (2 encoders, 2 decoders).
Field drop found in this group
ParquetSource::metadata_size_hintis not serialized.ParquetScanExecNodecarriesbase_conf,predicateandparquet_optionsonly, and the hint is not part ofTableParquetOptionseither, so a source configured with the publicwith_metadata_size_hintloses its footer prefetch size on round-trip.Several other
ParquetSourcefields are unmentioned by the encoder and need an explicit decision:parquet_file_reader_factoryandencryption_factorycannot be serialized and are rebuilt from the decode context (that is already documented ontry_from_proto-- it should become a_binding with the same note), whilereverse_row_groupsandsort_order_for_reorderneed checking against how they are set.Why
Serde hooks that read state through getters or
self.fieldmake an added field invisible to serialization: nothing breaks at compile time, the field simply stops round-tripping, andDebug-comparing round-trip tests do not notice.HashJoinExec::fetchwas 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-planplan nodes are done.physical-exprand thedatasource*crates were never converted.What to do
For each hook in scope:
try_to_proto, start with an exhaustivelet 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).try_from_proto, destructure the prost-generated node struct the same way, so adding a field todatafusion.protois a compile error in every decoder.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.
Part of EPIC: destructure proto serde hooks in the
datasourcecrates #24612.