sarutak commented on code in PR #56378:
URL: https://github.com/apache/spark/pull/56378#discussion_r3378282944
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/txns.scala:
##########
@@ -279,6 +279,10 @@ class SharedTablesInMemoryRowLevelOperationTableCatalog
super.initialize(name, options)
tables = SharedTablesInMemoryRowLevelOperationTableCatalog.sharedTables
}
+
+ // Return the live table instance (not a snapshot copy) so that TRUNCATE and
DROP
+ // mutations affect the shared catalog state immediately.
+ override def loadTable(ident: Identifier): Table = liveTable(ident)
Review Comment:
As you say, there's a design gap that `TRUNCATE TABLE` and `DROP TABLE` are
write operations but are resolved via `UnresolvedTable` -> `lookupTableOrView`
-> `loadTable(ident)` (the read-only overload), rather than going through
`loadTable(ident, writePrivileges)`.
However, I believe the current fix is appropriate for the following reasons.
1. No production impact: Real-world catalogs (Hive, Delta, Iceberg) return
the same live table instance from both `loadTable` overloads. The copy-on-read
behavior is specific to this test catalog, introduced by SPARK-56995 for
transaction cache validation.
2. Scope of the root fix: Propagating write semantics through
`UnresolvedTable` would require changes across the Parser, Analyzer resolution
rules, and the `DROP TABLE` path (which uses `UnresolvedIdentifier`, a separate
mechanism). I think it's disproportionate for a problem that only manifests in
a test utility class.
3. Semantic fit: `SharedTablesInMemoryRowLevelOperationTableCatalog` exists
to share mutable state across operations within a test. Returning the live
instance aligns with that purpose. The parent class's snapshot behavior serves
a different need (transaction isolation testing), which `SharedTables` callers
don't require.
So this is a targeted fix for the test infrastructure, not a bandaid over a
production bug.
--
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]
