peter-toth commented on code in PR #59997:
URL: https://github.com/apache/spark/pull/59997#discussion_r4436844961
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetReadSupport.scala:
##########
@@ -106,13 +106,48 @@ class ParquetReadSupport(
new ParquetRecordMaterializer(
parquetRequestedSchema,
ParquetReadSupport.expandUDT(catalystRequestedSchema),
- new ParquetToSparkSchemaConverter(conf),
+ new ParquetReadSchemaConverter(conf, parquetRequestedSchema, fileSchema),
convertTz,
datetimeRebaseSpec,
int96RebaseSpec)
}
}
+/**
+ * Interpret LISTs using the file schema. Clipping a repeated group to one
field can make a
+ * two-level LIST indistinguishable from a three-level LIST in the requested
schema.
+ */
+private[parquet] class ParquetReadSchemaConverter(
+ conf: Configuration,
+ requestedSchema: MessageType,
+ fileSchema: MessageType) extends ParquetToSparkSchemaConverter(conf) {
+
+ private lazy val fileRepeatedTypes = {
+ // Equal requested types at different paths can have different layouts in
the file.
+ val types = new util.IdentityHashMap[Type, Type]()
+ def visit(requested: GroupType, file: GroupType): Unit = {
+ if
(file.getLogicalTypeAnnotation.isInstanceOf[ListLogicalTypeAnnotation] &&
+ requested.getFieldCount == 1 && file.getFieldCount == 1) {
+ types.put(requested.getType(0), file.getType(0))
+ }
+ requested.getFields.asScala.foreach {
Review Comment:
**Finding 1.** None of the new tests reaches this recursion through a LIST.
Each of their LISTs is a top-level field or sits in a plain struct. A two-level
LIST inside another LIST is also the one shape I found where base returns a
wrong value instead of failing. I wrote this file, with one row whose
`outer[0].inner` is `[(1, 2), (3, 4)]`:
```
message m {
optional group outer (LIST) {
repeated group o {
optional group inner (LIST) {
repeated group i { optional int32 a; optional int32 b; }
}
optional int32 z;
}
}
}
```
Then I read `outer[0].inner[1].a` with the schema `outer ARRAY<STRUCT<inner:
ARRAY<STRUCT<a: INT, b: INT>>, z: INT>>`:
- base, row reader: `null`, with no error
- base, vectorized reader: `Invalid Spark read type: expected optional group
inner (LIST) ...`
- this PR: `3` in V1 and V2, with both readers
When I stopped this loop from descending into a LIST, all three new tests
still passed, but this query failed again in the vectorized reader. A test with
this shape next to the first one would pin the recursion.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaPruningSuite.scala:
##########
@@ -35,6 +41,89 @@ abstract class ParquetSchemaPruningSuite extends
SchemaPruningSuite with Adaptiv
override protected val vectorizedReaderNestedEnabledKey: String =
SQLConf.PARQUET_VECTORIZED_READER_NESTED_COLUMN_ENABLED.key
+ test("a pruned two-level Parquet LIST preserves its element type and field
IDs") {
Review Comment:
**Finding 3.** Minor: Spark regression tests usually start with the ticket,
e.g. `test("SPARK-60788: a pruned two-level Parquet LIST preserves its element
type and field IDs")`. The same goes for the other two new tests.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetReadSupport.scala:
##########
@@ -106,13 +106,48 @@ class ParquetReadSupport(
new ParquetRecordMaterializer(
parquetRequestedSchema,
ParquetReadSupport.expandUDT(catalystRequestedSchema),
- new ParquetToSparkSchemaConverter(conf),
+ new ParquetReadSchemaConverter(conf, parquetRequestedSchema, fileSchema),
convertTz,
datetimeRebaseSpec,
int96RebaseSpec)
}
}
+/**
+ * Interpret LISTs using the file schema. Clipping a repeated group to one
field can make a
+ * two-level LIST indistinguishable from a three-level LIST in the requested
schema.
+ */
+private[parquet] class ParquetReadSchemaConverter(
Review Comment:
**Finding 4.** Minor: this class splits `class ParquetReadSupport` from its
companion `object ParquetReadSupport`. Moving it after the object keeps the
pair together.
--
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]
