LuciferYang commented on PR #58340: URL: https://github.com/apache/spark/pull/58340#issuecomment-5474356468
Thanks, @peter-toth. All four are in at `b6c4b7b748b`, and CI is green on it. **13.** In this PR rather than as a follow-up. I took your clause and added the second disqualifier from `FileTable`'s scaladoc, so the javadoc now names both reasons a built-in source is held back here: a parser that decides what counts as a malformed record from the set of columns it was asked for, and a source that resolves a column by its position in the projection. Those two are the whole gap between the two texts, and the capability is unreleased, so it is cheaper here than after. **14.** Dropped rather than inverted, as you suggested. `rows` loses its `useV1` parameter, the three V1 expectations and the FAILFAST `intercept` go with it, and the comments now say what a merged scan would do instead of what V1 does. Gap 4 is rewritten around CSV and JSON getting no merging at all being deliberate, with the `[8, 80]` numbers attributed to the V1 path and to SPARK-59107 fixing them there, after which both paths decline these shapes alike. The order is in all three descriptions now: #58411, then this, then #58409. One correction to the finding. "Every `useV1 = true` expectation here" does not hold for the two parquet ones: this suite pins `ignoreCorruptFiles` and `ignoreMissingFiles` to false in `sparkConf`, and parquet's parser does not depend on the projection, so #58411's gate never fires for them. Measured the way you did it, with this suite and the helper copied into a worktree at #58411's head: the CSV and JSON test passes there, and so does "V1 merges differing partition filters, V2 does not". So only the csv and json arms had to go, and the V1/V2 parity pair stays. **15.** Split in two. Dropping `&& hasStrictFileReads` now fails both, the capability one at its first assertion and the rows one with correct answer `[45, 0]` against Spark answer `[null, 0]`, which is the number your scratch suite reported, now in a test that can report it. The three-overrides mutation is 8 of 16, and the eight that pass are six declines, the partition-filter test whose V2 side declines under the mutation too, and the new rows test, whose separate scans the mutation also produces. **16.** In the description, since `FileSourceOptions.hasStrictFileReads` is not in master yet. Whichever way the two land, this ends up calling it. Two more measurements on the sequencing, since #58409 belongs to the same set. Its five commits cherry-picked onto this head leave the `AvroV2Suite` case passing, so this PR and #58409 do not interact beyond the gate. And `git merge-tree` puts the only textual conflict among the three in `connector/avro/src/test/scala/org/apache/spark/sql/avro/AvroSuite.scala`, between this one and #58411, both of which append a test there, so whichever lands second needs a rebase. -- 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]
