andygrove commented on code in PR #5654:
URL: https://github.com/apache/datafusion-comet/pull/5654#discussion_r4094901006
##########
native/core/src/parquet/cast_column.rs:
##########
@@ -270,12 +284,22 @@ impl PhysicalExpr for CometCastColumnExpr {
let input_physical_field = self.input_physical_field.data_type();
let target_field = self.target_field.data_type();
+ // Relabeling only swaps metadata, so it is right when every requested
field reads
+ // the file field at its own position. A mapping that reorders fields
(ids resolved
+ // to other positions) has to go through the nested conversion below.
+ let positional = self
Review Comment:
This gate fixes a silent wrong-result bug on `main`, and I filed #6192 for
it. Your summary covers swapped ids, but the shape I'd expect users to actually
hit is a nested column dropped and added back under the same name. A file with
`s struct<x (id 1), y (id 2)>` read as `s struct<x (id 3), y (id 2)>` returns
the old `x` values on `main`, where Spark returns null. The same happens inside
a list element and a map value. All of those match Spark 4.1.3 on this branch,
with the native scan in the plan.
Could you add `Closes #6192` to the description, plus a `ParquetReadSuite`
test that runs the drop-and-re-add read through `checkSparkAnswerAndOperator`
for a struct, a list element and a map value? At the moment only
`test_swapped_field_ids_bypass_relabel_shortcut` covers this, at the expression
level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
