cloud-fan commented on code in PR #58462:
URL: https://github.com/apache/spark/pull/58462#discussion_r3920864969
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala:
##########
@@ -510,14 +525,31 @@ private[sql] object CatalogV2Util {
timeTravelSpec: Option[TimeTravelSpec] = None,
writePrivilegesString: Option[String] = None,
options: CaseInsensitiveStringMap = CaseInsensitiveStringMap.empty()):
Table = {
+ val stateOptions = extractTableStateOptions(catalog, options)
+ getTableWithStateOptions(
+ catalog, ident, stateOptions, timeTravelSpec, writePrivilegesString)
+ }
+
+ /**
+ * Loads a table using table-state options already projected by the caller.
+ */
+ def getTableWithStateOptions(
+ catalog: CatalogPlugin,
+ ident: Identifier,
+ stateOptions: CaseInsensitiveStringMap,
+ timeTravelSpec: Option[TimeTravelSpec] = None,
+ writePrivilegesString: Option[String] = None): Table = {
val timeTravel: TimeTravel = timeTravelSpec match {
case Some(v: AsOfVersion) => new TimeTravel.AsOfVersion(v.version)
case Some(ts: AsOfTimestamp) => new
TimeTravel.AsOfTimestamp(ts.timestamp)
case None => null
}
val context = new TableContext(timeTravel,
parseWritePrivileges(writePrivilegesString))
- val stateOptions = extractTableStateOptions(catalog, options)
- catalog.asTableCatalog.loadTable(ident, context, stateOptions)
+ // Callers may retain stateOptions as a cache key.
CaseInsensitiveStringMap exposes mutable
+ // collection views, so do not let catalog code mutate the retained
instance.
+ val catalogStateOptions =
+ new CaseInsensitiveStringMap(stateOptions.asCaseSensitiveMap())
Review Comment:
**Blocking (P1):** `stateOptions.asCaseSensitiveMap()` can contain both
original spellings for a case-insensitive key, while lookup and equality use
one normalized delegate value. Reconstructing from the case-sensitive map
replays those duplicates in a fresh map order, so the copy can choose a
different effective value. A Connect named-table read can carry both `snapshot`
and `SNAPSHOT`; Spark may then load one snapshot but cache it under the other
state key, allowing a later unambiguous reference to reuse the wrong table.
Please copy from the effective entries (for example, `new
CaseInsensitiveStringMap(stateOptions)`) and add a duplicate-case regression.
--
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]
