LuciferYang commented on PR #58340: URL: https://github.com/apache/spark/pull/58340#issuecomment-5454199423
Thanks, this is all taken. The change is now what you proposed: `FileTable.CAPABILITIES` no longer carries `SCAN_MERGING`, a `CAPABILITIES_WITH_SCAN_MERGING` sits beside it, and a `protected def supportsScanMerging: Boolean = false` seam picks between them. `ParquetTable`, `OrcTable`, `TextTable` and `AvroTable` override it; `CSVTable` and `JsonTable` do not. Pushed as 7dd18334f98. 1. The migration-guide entry is gone rather than corrected. With CSV and JSON out there is no user-visible result change left to document. 2. Conceded above, and now pinned rather than argued. The parse-mode test asserts all three of your shapes on both read paths: `DROPMALFORMED` `[10, 80]` on V2 against `[8, 80]` on V1, `PERMISSIVE` with `_corrupt_record` `[0, 80]` against `[1, 80]`, and `FAILFAST` with the short CSV row returning rows on V2 and throwing on V1. 3. The description is rewritten against the current diff, including the helper, the `DSv2PlanMergingSuite` change and the Avro test. Your correction to the count stands: rerun on the current suite it is 6 of 13 plus the Avro test, and of the 7 that still pass, 4 assert a decline, one asserts different values on the two paths, and two are the new CSV and JSON tests. 4. Taken. `assertUsesFileSourceV1` is the mirror you sketched, and both `mergedCounts` and the parse-mode helper call one or the other. I also took the `subqueryCounts` suggestion for `rows`, which earned itself: asserting before `collect()` reported `(2, 0)` on V1, because AQE had not finalized and the reuse was not yet in the plan, so it moved after. 5. Taken as written, including the name. 6. Taken. One line added to the criterion, because the `FAILFAST` measurement taken alone proves more than it should: a corrupt column chunk, or `datetimeRebaseModeInRead=EXCEPTION` meeting an ancient value, makes any format throw on a column the narrower scan had pruned, so "merging can surface a new error" would empty the list. What separates CSV and JSON is the silent and unrecoverable change to row membership and row content, and `FAILFAST` is that same projection dependence surfacing as an error. The four override comments say it that way, so a later reader does not ask why Parquet is not out too. 7. Taken, with your wording. That leaves the V1 side, which I will file separately rather than sequence this behind. Fixing it needs a signal on `FileFormat` plus plumbing so the decline can see whether the projections differ, which the leaf arm cannot: only `MergeContext` threads down and both leaves carry the full schema. It also changes behaviour users have today. Your reading that V1 is the bug rather than the target is what the fourth gap in the description now says. -- 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]
