LuciferYang commented on code in PR #58340:
URL: https://github.com/apache/spark/pull/58340#discussion_r3880092918
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileTable.scala:
##########
@@ -181,5 +181,9 @@ abstract class FileTable(
}
object FileTable {
- private val CAPABILITIES = util.EnumSet.of(BATCH_READ, BATCH_WRITE)
+ // A file table meets the determinism contract SCAN_MERGING requires:
`fileIndex` is a lazy val,
+ // so every scan built from this table lists the same files, and
`newScanBuilder` returns a fresh
+ // builder over `mergedOptions(options)`. The same options, pushed filters
and pruned columns
+ // therefore rebuild an equivalent scan.
+ private val CAPABILITIES = util.EnumSet.of(BATCH_READ, BATCH_WRITE,
SCAN_MERGING)
Review Comment:
@peter-toth thanks for measuring this, and sorry for the wrong table above.
Both shapes reproduce on this head, on V1 as well as V2:
| shape | merged | `MergeSubplans` excluded |
|---|---|---|
| `PERMISSIVE`, `_corrupt_record` in schema, csv and json | `[1, 80]` | `[0,
80]` |
| `FAILFAST`, short CSV row | throws | `[10, 80]` |
My table measured one malformed shape and I generalized it. The `FAILFAST`
reasoning I gave only covers malformedness belonging to a column; a token-count
mismatch is a property of how many columns are parsed, and that is what the
merge changes. With the short row the `a`-only scan parses one column against
one token, so nothing is malformed until the merge makes it two.
That removes my objection. I weighed one non-default mode against V1 parity;
it is a different trade when the default mode changes results and `FAILFAST`
can turn a working query into an error. Your point that the superset clause is
the soundness argument rather than decoration is well taken.
Finding 1 is right too: V1 returns `[8, 80]` as well, so falling back to
`useV1SourceList` restores nothing. Only excluding `MergeSubplans` does.
I will fix the guide sentence and the same claim in my summary comment, and
come back on the shape of the fix including your `supportsScanMerging` seam.
--
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]
