peter-toth commented on code in PR #58409:
URL: https://github.com/apache/spark/pull/58409#discussion_r3911761052
##########
connector/avro/src/main/scala/org/apache/spark/sql/v2/avro/AvroPartitionReaderFactory.scala:
##########
@@ -106,7 +106,8 @@ case class AvroPartitionReaderFactory(
avroFilters,
options.useStableIdForUnionType,
options.stableIdPrefixForUnionType,
- options.recursiveFieldMaxDepth)
+ options.recursiveFieldMaxDepth,
+ dataSchema = Some(dataSchema))
Review Comment:
**Finding 6.** Both prerequisites landed while this was open, so "it should
land after them and remove them in one commit" now applies to this commit:
#58340 is `484866bd80d` on `master` and `69e1cb10f43` on `branch-4.x`, #58411
is `b82f9872d1c` and `d9176bf31be`. The merge at this head brings both in, and
the diff still leaves both gates standing. Nothing fails while they stand,
which is why it is easy to miss.
The part that makes this more than a tidy-up is the documentation.
`docs/sql-performance-tuning.md:343` lists "`avro` read with
`positionalFieldMatching`, which pairs a column with the Avro field at its
position in that schema" among the projection-sensitive V1 relations, and
`:345` ends with "Avro withholds it under `positionalFieldMatching`, which
resolves a column by its position in the projection". Neither sentence survives
this fix, since the position is the data schema's. Those two paragraphs have to
change in this PR whether or not the predicates come off with them.
The rest, in the order I would do it:
- `AvroTable.scala:67` — `supportsScanMerging` becomes `override protected
def supportsScanMerging: Boolean = true`, not a deleted override. `FileTable`'s
default is `false`, so dropping it would take the capability away from Avro V2
altogether. The second paragraph of the comment above it goes, the first one
("Avro has no record-level parse verdict") stays, and `AvroOptions` leaves the
import.
- `DataSourceUtils.scala:203-205` — drop the `case _: AvroFileFormat` arm.
`options` is then unused, so `hasProjectionSensitiveParser` loses that
parameter and `:193` becomes `hasProjectionSensitiveParser(hs.fileFormat)`. The
`org.apache.spark.sql.avro` import at `:32` goes with it, and the doc sentence
at `:179-181` loses the Avro clause together with its "(SPARK-59108, which
removes that at the root, so this case goes with it)".
- `AvroSuite.scala:3873` — the `AvroV1Suite` test "SPARK-59107:
positionalFieldMatching makes an avro read projection-sensitive" goes. Its
`scanColumns === Seq(Seq("a"), Seq("b"))` fails once the arm is gone. That is
the one piece of this CI catches for you; everything above it is silent.
- `AvroSuite.scala:4128-4130` — the
`!positional.capabilities().contains(SCAN_MERGING)` assertion in the
SPARK-57205 test, plus the first two sentences of its comment. Keep the
`withSQLConf` block and the `hasStrictFileReads` sentence.
`FileTable`'s class doc names the same shape ("neither does one that
resolves a column by its position in the projection") as a general statement of
the contract, so that one can stay.
Both prerequisites are also on `branch-4.x`, so a backport of this fix has
to carry the same removal there.
--
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]
