LuciferYang commented on code in PR #58340:
URL: https://github.com/apache/spark/pull/58340#discussion_r3878158743
##########
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
Review Comment:
Thanks for tracing this through. The mechanism is right: canonical equality
does not distinguish two table instances whose listings were taken at different
times, because the table case classes exclude the lazy `fileIndex` and
`InMemoryFileIndex.equals` compares root paths only.
On the wording, I read the comment as already staying inside instance scope:
`fileIndex` is a lazy val "so every scan built from this table lists the same
files", and the sentence after it is about rebuilding a scan from the same
builder. I would rather not add a cross-instance caveat, for the reason you
gave: two scans over the same paths reading the same columns were already
collapsed before this change, by the identical-plan path and by
`ReuseSubquery`/`ReuseExchange`, and V1 collapses them too. A caveat here would
document `InMemoryFileIndex`'s listing semantics rather than anything the
capability introduces, and it would be easier to find next to that equality.
One note: the comment has moved since you read it. The contract now lives in
a class-level scaladoc on `FileTable` addressed to subclass authors, and what
is left at `CAPABILITIES` is the two facts that make it hold for the built-in
tables. If the new text still reads as overclaiming, point at the clause and I
will fix it.
--
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]
