cloud-fan commented on code in PR #58295:
URL: https://github.com/apache/spark/pull/58295#discussion_r3901259969
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V2TableUtil.scala:
##########
@@ -130,7 +135,41 @@ private[sql] object V2TableUtil extends SQLConfHelper {
val originMetaSchema = CatalogV2Util.toStructType(originMetaCols)
val metaCols = filter(originMetaColNames, metadataColumns(table))
val metaSchema = CatalogV2Util.toStructType(metaCols)
- SchemaUtils.validateSchemaCompatibility(originMetaSchema, metaSchema,
resolver, mode, checkIds)
+ val schemaErrors = SchemaUtils.validateSchemaCompatibility(
+ originMetaSchema, metaSchema, resolver, mode, checkIds)
+ schemaErrors ++ shadowedMetadataColumnErrors(table, metaCols)
+ }
+
+ /**
+ * Reports captured metadata columns that a data column of the same name now
hides.
+ *
+ * When a data column takes a metadata column's name, a connector that does
not rename the
+ * conflict (`canRenameConflictingMetadataColumns` is false) suppresses the
metadata column via
+ * `metadataOutputWithOutConflicts`. A suppressed metadata column can no
longer be resolved, so a
+ * captured reference to it is broken, and on a partially pruned scan it
silently reads the data
+ * column's values instead. The `SupportsMetadataColumns` contract advises a
non-renaming source
+ * to reject such a data-column name but does not enforce it, so this
reports the conflict
+ * rather than leaving it silent.
+ */
+ private def shadowedMetadataColumnErrors(
+ table: Table,
+ reportedMetaCols: Seq[MetadataColumn]): Seq[String] = {
+ if (reportedMetaCols.isEmpty || renamesConflictingMetadataColumns(table)) {
+ Nil
+ } else {
+ val dataColNames = table.columns.iterator.map(c =>
normalize(c.name)).toSet
Review Comment:
**Blocking (P1):** The planner decides metadata/data conflicts with the
configured SQL resolver, but this check uses `toLowerCase(Locale.ROOT)`. Those
differ for valid Unicode identifiers: in the default case-insensitive mode,
`"\u0130ndex".equalsIgnoreCase("index")` is true, while their root-locale
lowercase strings are unequal (`i\u0307ndex` versus `index`). A captured
metadata column can therefore pass this validation even though fresh resolution
hides it, then reach name-based scan/output reconciliation and return the data
column's values. Please compare the names with the same `resolver` used by the
planner and add a Unicode regression test.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/V2TableUtil.scala:
##########
@@ -130,7 +135,41 @@ private[sql] object V2TableUtil extends SQLConfHelper {
val originMetaSchema = CatalogV2Util.toStructType(originMetaCols)
val metaCols = filter(originMetaColNames, metadataColumns(table))
val metaSchema = CatalogV2Util.toStructType(metaCols)
- SchemaUtils.validateSchemaCompatibility(originMetaSchema, metaSchema,
resolver, mode, checkIds)
+ val schemaErrors = SchemaUtils.validateSchemaCompatibility(
+ originMetaSchema, metaSchema, resolver, mode, checkIds)
+ schemaErrors ++ shadowedMetadataColumnErrors(table, metaCols)
+ }
+
+ /**
+ * Reports captured metadata columns that a data column of the same name now
hides.
+ *
+ * When a data column takes a metadata column's name, a connector that does
not rename the
+ * conflict (`canRenameConflictingMetadataColumns` is false) suppresses the
metadata column via
+ * `metadataOutputWithOutConflicts`. A suppressed metadata column can no
longer be resolved, so a
Review Comment:
**Nit (P3):** This explanation says the already-captured metadata attribute
is suppressed and unresolvable, but `metadataOutputWithOutConflicts` returns
metadata attributes already present in `relation.output`, and refresh preserves
that output via `r.copy(table = currentTable)`. The wrong-value path occurs
later when `PushDownUtils.toOutputAttrs` reconciles the refreshed scan schema
to retained output attributes by physical name, which can bind the same-named
data field to the metadata attribute. Please reword this comment so it
describes that mechanism.
--
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]
