andygrove opened a new issue, #6994:
URL: https://github.com/apache/datafusion-comet/issues/6994
### Describe the bug
With `spark.sql.parquet.fieldId.read.enabled=true`, take a requested group
in which one field selects a file column by Parquet field id, and a sibling
without an id has the same folded name as that column. Spark's
`clipParquetGroupFields` resolves each requested field on its own, so both
fields select the same file column. Comet then returns different results at the
root and inside a struct.
Take a file column `A` (id 1) holding `1, 2, 3`, read with requested fields
`x` (id 1) and `a` (no id). Here is what the read returns for `a`:
| Reader | Root columns | Fields of a struct `s` |
|---|---|---|
| Spark vectorized reader (the default) | garbage that changes between runs
(`0`, `1846643200`, `719301120`, ...) | garbage that changes between runs |
| Spark parquet-mr reader | null | null |
| Comet native scan | null | `1, 2, 3`, the same values as `x` |
`x` reads `1, 2, 3` in every case except the vectorized reader with `a`
listed first. That reader gives the column to whichever field comes first and
garbage to the other. The parquet-mr reader and Comet's root give the column to
the id match in either order. A root field that matches `A` exactly, rather
than case-insensitively, gives the same results.
The two Comet levels differ because of how each one consumes the resolution:
- The root renames file columns so DataFusion's exact-name lookup finds
them. A file column can carry only one name, so `remap_physical_schema` gives
`A` to the id match (`RootResolution::IdMatch` outranks `NameMatch`), and `a`
reads as a missing column.
- The nested path, `match_struct_fields`, is used by
`parquet_convert_struct_to_struct` and by the plan-time conversion check. It
returns a file field index for each requested field, so `x` and `a` both read
`A`.
Since #6990, both paths get identical selections from
`FieldResolver::select`. They differ only in what happens to a file column that
two requested fields select.
On the Spark side, the parquet-mr answer comes from `ParquetRowConverter`.
It maps the clipped Parquet fields back to requested fields by id first. Both
clipped copies of `A` carry id 1, so both map to `x`, and `a` never gets a
converter. The vectorized reader builds a column reader for each requested
field, and both resolve to the same column chunk. My unconfirmed guess is that
the second reader shares a page reader the first one has already drained.
### Steps to reproduce
In a `CometTestBase` suite on the default Spark 4.1.3 profile:
```scala
val id1 = new MetadataBuilder().putLong(ParquetUtils.FIELD_ID_METADATA_KEY,
1L).build()
spark
.createDataFrame(
spark.sparkContext.parallelize(Seq(Row(1), Row(2), Row(3))),
StructType(Seq(StructField("A", IntegerType, nullable = true, id1))))
.write
.parquet(path)
withSQLConf(SQLConf.PARQUET_FIELD_ID_READ_ENABLED.key -> "true") {
spark.read
.schema(StructType(Seq(
StructField("x", IntegerType, nullable = true, id1),
StructField("a", IntegerType, nullable = true))))
.parquet(path)
.show()
}
```
Run it with Comet's native scan, with `spark.comet.enabled=false`, and with
`spark.comet.enabled=false` plus
`spark.sql.parquet.enableVectorizedReader=false`. For the nested case, write
the same column inside a struct `s` (id 10), and request `s` with the fields
`x` (id 1) and `a`.
### Expected behavior
Comet should return the same result at the root and inside a struct. Spark's
vectorized reader has no stable answer here, so the target is presumably the
parquet-mr reader's: the id match reads the column, and the name match reads
null, at both levels. The root already does this. Getting the same result from
`match_struct_fields` would mean giving each file field to its strongest
claimant only. One way is to move the root's claim precedence from
`schema_adapter.rs` into `FieldResolver`, so both paths share it. Another is to
reject the read at both levels.
### Additional context
I found this while verifying #6990, which closes #6978. Comet behaved this
way before #6990. That PR keeps the behavior and documents the root side on
`RootResolution`.
The shape needs field-id reads and a requested group that mixes fields with
and without ids. Iceberg and Delta assign ids to every column, so it is
unlikely in practice. Spark's vectorized reader returning garbage for this
shape is a Spark bug in its own right.
--
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]
