Describe the bug
The physical plan protobuf encoding for CSV and JSON scans silently drops several
scan options. They are not represented in CsvScanExecNode / JsonScanExecNode
(or in the shared FileScanExecConf), so encoding succeeds without error and the
decoded plan reads the data differently from the original:
| Option |
Encoded? |
Decodes as |
JsonSource::newline_delimited |
no |
always true (NDJSON) |
CsvSource::terminator |
no |
None (default \n) |
FileScanConfig::file_compression_type (CSV, JSON) |
no |
UNCOMPRESSED |
For newline_delimited and file_compression_type this is a correctness problem,
not just a lost setting: a JSON-array file decoded as NDJSON, or a gzipped file
decoded as uncompressed, produces wrong results or a parse error on the remote
node rather than a serialization failure at the sender.
This is pre-existing behavior, not a regression. It was surfaced while migrating
these sources onto the per-plan proto hooks (#23494), where it is now recorded in
doc comments on CsvSource::try_from_proto and JsonSource::try_from_proto
(#24177, #24178).
To Reproduce
Round-trip a DataSourceExec through PhysicalPlanNode encode/decode with any of:
JsonSource::new(schema).with_newline_delimited(false)
CsvSource::new(schema).with_terminator(Some(b'#'))
FileScanConfigBuilder::new(url, source).with_file_compression_type(FileCompressionType::GZIP)
Each round-trips back to the default value.
Expected behavior
Either of:
- Add the missing fields to the proto messages (
optional bool newline_delimited
on JsonScanExecNode, optional string terminator on CsvScanExecNode, and a
compression field for CSV/JSON) and encode/decode them. Making them optional
keeps the wire format backward compatible: an absent field decodes to today's
default.
- At minimum, return an error from
try_to_proto when a non-default value is set,
so the loss is loud instead of silent.
(1) is preferable — the options are cheap to encode and (2) turns a working local
plan into a hard failure the moment it is serialized.
Additional context
Related: #23494 (proto hook migration EPIC), #23516, #24177, #24178.
Describe the bug
The physical plan protobuf encoding for CSV and JSON scans silently drops several
scan options. They are not represented in
CsvScanExecNode/JsonScanExecNode(or in the shared
FileScanExecConf), so encoding succeeds without error and thedecoded plan reads the data differently from the original:
JsonSource::newline_delimitedtrue(NDJSON)CsvSource::terminatorNone(default\n)FileScanConfig::file_compression_type(CSV, JSON)UNCOMPRESSEDFor
newline_delimitedandfile_compression_typethis is a correctness problem,not just a lost setting: a JSON-array file decoded as NDJSON, or a gzipped file
decoded as uncompressed, produces wrong results or a parse error on the remote
node rather than a serialization failure at the sender.
This is pre-existing behavior, not a regression. It was surfaced while migrating
these sources onto the per-plan proto hooks (#23494), where it is now recorded in
doc comments on
CsvSource::try_from_protoandJsonSource::try_from_proto(#24177, #24178).
To Reproduce
Round-trip a
DataSourceExecthroughPhysicalPlanNodeencode/decode with any of:Each round-trips back to the default value.
Expected behavior
Either of:
optional bool newline_delimitedon
JsonScanExecNode,optional string terminatoronCsvScanExecNode, and acompression field for CSV/JSON) and encode/decode them. Making them optional
keeps the wire format backward compatible: an absent field decodes to today's
default.
try_to_protowhen a non-default value is set,so the loss is loud instead of silent.
(1) is preferable — the options are cheap to encode and (2) turns a working local
plan into a hard failure the moment it is serialized.
Additional context
Related: #23494 (proto hook migration EPIC), #23516, #24177, #24178.