viirya commented on code in PR #59997:
URL: https://github.com/apache/spark/pull/59997#discussion_r4437931782
##########
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:
+1. A two-level LIST as a MAP value hits the same recursion: `mp MAP<STRING,
ARRAY<STRUCT<a, b>>>` with `mp['k'][1].a` fails on base with
`PARQUET_COLUMN_DATA_TYPE_MISMATCH` and passes with this PR. One test with
either shape would pin it.
##########
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 = {
Review Comment:
nit: `lazy` doesn't seem needed here, since the converter is used as soon as
it is built.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetRowConverter.scala:
##########
@@ -254,19 +255,20 @@ private[parquet] class ParquetRowConverter(
// field has id, try to match by id first before falling back to match
by name
catalystFieldIdxByFieldId.get(fieldId.intValue())
}.orElse {
- // field doesn't have id, just match by name
- catalystFieldIdxByName.get(parquetField.getName)
+ // A field read only for struct nullness can share a name with a
missing field ID.
Review Comment:
Could this comment state the invariant instead? For example: "A field with
an ID is matched only by ID, as in `ParquetReadSupport.clipParquetGroupFields`.
A name match here can only be a column read for struct nullness." That makes it
clear the filter must stay in sync with clipping.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaPruningSuite.scala:
##########
@@ -70,6 +159,23 @@ class ParquetV2SchemaPruningSuite extends
ParquetSchemaPruningSuite {
.sparkConf
.set(SQLConf.USE_V1_SOURCE_LIST, "")
+ test("row reader uses the scan's field ID setting") {
Review Comment:
Is this V2-only because V1 ignores the data source option, so the session
value decides both clipping and conversion? In V1 I get the same results before
and after this PR. A one-line comment would save the next reader from checking.
--
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]
