You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/10/15 15:20:47 UTC
[PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
andygrove opened a new pull request, #59510:
URL: https://github.com/apache/spark/pull/59510
### What changes were proposed in this pull request?
With `spark.sql.parquet.fieldId.read.enabled=true`, `ParquetReadSupport.clipParquetGroupFields` resolves each requested field on its own: a field with a field id by id, and any other field by name. Two requested fields can therefore be clipped to the same Parquet field, and neither reader can read a column twice (see below). This PR makes such a read fail with a clear error instead of returning wrong values:
- `clipParquetGroupFields` throws the new `PARQUET_FIELD_REQUESTED_MORE_THAN_ONCE` error when two or more requested fields of a struct are clipped to the same Parquet field. That covers a field id match plus a name match, and two fields with the same id, at the top level and inside nested structs. The check is in the clipping code that both readers use, so it covers the vectorized reader and the parquet-mr reader.
- `VectorizedColumnReader.readBatch` throws a `ParquetDecodingException` when a column without repeated values runs out of pages with rows still to read, instead of returning. With the check above nothing in Spark reaches this, but returning leaves the rest of the vector unwritten without any signal, so this keeps a column that is ever read twice, or a truncated column chunk, from quietly returning wrong values. Repeated columns still stop at the end of the pages, because the end of a list is not known until then.
### Why are the changes needed?
Take a file column `A` with field id 1. A requested `x` with id 1 selects it by id, and a requested `a` without an id selects it by name, so the requested Parquet schema lists `A` twice. Neither reader returns `A`'s values for both fields, and neither reports an error:
- The vectorized reader builds one `VectorizedColumnReader` per requested leaf, but a Parquet page store has a single queue of pages per column. The first reader drains it. For the second, `readPage()` returns -1 and `readBatch` stops as if it had read every page, so its vector is never written, and its rows read as non-null `0` with on-heap vectors, or as whatever the memory holds with off-heap vectors.
- The parquet-mr reader (`spark.sql.parquet.enableVectorizedReader=false`) has `ParquetRowConverter` map the clipped Parquet fields back to the requested fields by id first. Both copies of `A` carry id 1, so both map to `x`, and `a` reads null.
```scala
val id1 = new MetadataBuilder().putLong("parquet.field.id", 1L).build()
spark.createDataFrame(
spark.sparkContext.parallelize(Seq(Row(1), Row(2), Row(3)), 1),
StructType(Seq(StructField("A", IntegerType, nullable = true, id1))))
.write.parquet("/tmp/field-id-twice")
spark.conf.set("spark.sql.parquet.fieldId.read.enabled", "true")
val x = StructField("x", IntegerType, nullable = true, id1)
val a = StructField("a", IntegerType, nullable = true)
spark.read.schema(StructType(Seq(x, a))).parquet("/tmp/field-id-twice").show()
```
Before this change, `x` reads 1, 2, 3 and `a` reads 0, 0, 0 with the vectorized reader (the other way round when `a` comes first), and `a` reads null with the parquet-mr reader. A struct holding `x` and `a` behaves the same.
Reading both fields correctly would take changes to how both readers map clipped fields back to the requested ones, and a way for the vectorized reader to serve one column to several leaves. This PR is the smaller step of replacing the silent wrong results with an error. Field-id reads (SPARK-38094) have returned these results since 3.3.
### Does this PR introduce _any_ user-facing change?
Yes. Reading a schema in which two requested fields select the same Parquet column, which returned wrong values without an error (see above), now fails with `PARQUET_FIELD_REQUESTED_MORE_THAN_ONCE`. The wrong results come from field-id reads, so released versions since 3.3 have them as well. Reads that do not select a column more than once are not affected.
### How was this patch tested?
New tests:
- `ParquetFieldIdSchemaSuite`: `ParquetReadSupport.clipParquetSchema` fails, naming the requested fields and the Parquet field, for an id match plus a name match in either order (case-sensitive and not), two fields with the same id, a third field matching by name, and a nested struct. It does not fail when the fields select different Parquet fields, or none.
- `ParquetFieldIdIOSuite`: reads with the vectorized and the parquet-mr reader fail for the same shapes (top level, nested, same id), and a requested field that matches no column still reads as null.
- `ParquetVectorizedSuite`: a requested schema that lists a column twice, over a page store that shares one page reader per column as Parquet's does, fails in `VectorizedColumnReader.readBatch` instead of leaving the second vector unwritten.
The new tests fail without this change (no exception is thrown, and the data reads back wrong) and pass with it. The `ParquetVectorizedSuite` test also fails when only the `readBatch` check is removed. Also ran, all passing: the packages `org.apache.spark.sql.execution.datasources.parquet` and `org.apache.spark.sql.execution.datasources.v2.parquet` with `FileBasedDataSourceSuite` (55 suites, 2075 tests passed, 7 ignored), and `SparkThrowableSuite`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (claude-sonnet-5-5)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6217306321
CI failure seems unrelated
```
- SPARK-46957: Migrated shuffle files should be able to cleanup from executor *** FAILED *** (1 minute, 36 seconds)
[info] The code passed to eventually never returned normally. Attempted 1012 times over 1.0003176462666667 minutes. Last failure message: 0 was not greater than or equal to 10.
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6220879839
Thank you, @andygrove and @peter-toth .
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth closed pull request #59510: [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column
URL: https://github.com/apache/spark/pull/59510
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6196605305
Thanks for the review, @peter-toth!
1. Added a migration guide entry under "Upgrading from Spark SQL 4.3 to 4.4" in `docs/sql-migration-guide.md` (9f9b807). It describes the new `PARQUET_FIELD_REQUESTED_MORE_THAN_ONCE` failure, the wrong results earlier versions returned, and the workaround of requesting the Parquet field once.
2. Updated SPARK-60262: added 3.5.9, 4.0.4, 4.1.3 and 4.2.0 as affected versions, and the `correctness` label.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6195683057
@peter-toth PTAL
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6219114929
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/f095057ea8de9f33fe816656cf31fbb20897ca3b
- merged into branch-4.x https://github.com/apache/spark/commit/4af89ec7760ceefc664fd6dc425bafce98258e1c
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60262][SQL] Fail Parquet reads where two requested fields select the same column [spark]
Posted by "peter-toth (via GitHub)" <gi...@apache.org>.
peter-toth commented on PR #59510:
URL: https://github.com/apache/spark/pull/59510#issuecomment-6219245530
Yes, that was an unrelated failure, other tests passed. Thanks for the fix @andygrove.
Can you please open backport PRs to `branch-4.0`, `branch-4.1`, `branch-4.2` and `branch-4.3`? I think you can drop the migration guide change from those.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org