cloud-fan commented on code in PR #58409:
URL: https://github.com/apache/spark/pull/58409#discussion_r3933800654
##########
sql/core/src/main/scala/org/apache/spark/sql/avro/AvroDeserializer.scala:
##########
@@ -452,15 +462,44 @@ private[sql] class AvroDeserializer(
}
}
+ /**
+ * The position of each `projection` field in `dataSchema`, which is what a
positional field match
+ * resolves against. Empty when there is no data schema to resolve against,
or when field matching
+ * is by name and the positions are unused.
+ *
+ * This takes a data schema position for an Avro field position, which
`recursiveFieldMaxDepth`
+ * can break: `SchemaConverters` drops a field it will not recurse into, so
the data schema is a
+ * gapped view of the Avro schema and every field after the gap resolves one
position early.
+ * Positional matching is already wrong for such a schema without this
method, because the fields
+ * after the gap shift by one whatever the projection is.
+ */
+ private def positionsInDataSchema(projection: StructType): Array[Int] =
dataSchema match {
+ case Some(schema) if positionalFieldMatch =>
+ projection.map(field => schema.fieldIndex(field.name)).toArray
Review Comment:
**Non-blocking (P2):** V1 file reads intentionally catch duplicate-column
validation and continue with a warning, so an explicit schema such as `(x, x)`
reaches this lookup. `StructType.fieldIndex` retains one index per name, which
makes both occurrences resolve to the same Avro ordinal: same-typed fields
silently return the second value twice, and differing types fail converter
construction. Please preserve each required field's source ordinal while the V1
scan boundary can still distinguish occurrences, carry that mapping into
top-level deserialization, keep nested records on local ordinals, and add a V1
regression with duplicate names and distinct physical values. V2 rejects
duplicate data-column names and needs no duplicate-schema change.
**Recommended change:** Preserve occurrence-specific source ordinals before
V1 required fields are reduced to StructField names, and pass that immutable
mapping through the normal and archive V1 reader paths into top-level Avro
deserialization.
**Why this works:** Capture each required attribute's ordinal in the full V1
data schema while duplicate occurrences are still distinguishable; have
AvroSchemaHelper consume those ordinals directly at the root, while nested
records continue to use their explicit local positions.
**Scope:** Avro V1 scan-pruning and normal/archive reader-to-deserializer
plumbing, plus one focused V1 duplicate-schema regression; no V2
duplicate-schema work.
**Compatibility:** Unique-name positional reads, default name-based
matching, V2 reads, and nested-record local matching retain their current
behavior; supported V1 duplicate-name positional reads regain
occurrence-correct values.
**Risks:** Capturing the mapping after attributes have already been reduced
to names would preserve the bug. The normal and archive V1 paths could diverge,
or root ordinal plumbing could accidentally affect nested-record local matching.
**Constraints:** Keep nested-record matching on explicit local positions. Do
not change default name-based matching or broaden the recursiveFieldMaxDepth
limitation. Do not add V2 duplicate-schema plumbing or coverage because V2
rejects duplicate data-column names.
**Success:** A V1 positional read with two same-named explicit-schema fields
and distinct physical values returns the two distinct values in order on both
applicable V1 reader paths, while existing unique-name and nested-record tests
remain unchanged.
--
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]
