[SPARK-59014][SQL] Reject a data column that hides a captured metadata column during DSv2 refresh validation - #58295
Open
yyanyy wants to merge 1 commit into
Open
Conversation
yyanyy
force-pushed
the
spark-dsv2-metacol-conflict-20260825
branch
3 times, most recently
from
August 26, 2026 00:53
234bfb2 to
80cd41e
Compare
yyanyy
marked this pull request as ready for review
August 26, 2026 01:15
…a column during DSv2 refresh validation `V2TableUtil.validateCapturedMetadataColumns` compared the captured metadata columns only against the metadata columns the connector still reports, so it could not see a conflict arriving on the data side. When a data column takes a captured metadata column's name and the connector suppresses the conflict (the default `canRenameConflictingMetadataColumns == false`), `metadataOutputWithOutConflicts` drops the metadata column: a captured reference to it becomes unresolvable, and on a partially-pruned scan `PushDownUtils.toOutputAttrs` collapses the two same-named fields so a query for the metadata column silently returns the data column's values. Detect that collision in the shared validator and report a user-facing error (`INCOMPATIBLE_TABLE_CHANGE_AFTER_ANALYSIS.METADATA_COLUMNS_MISMATCH`). Only the suppressed case is rejected; when the connector renames the conflicting metadata column it stays reachable and keeps working. The scan is skipped entirely when no captured metadata column survives the still-reported filter, so relations that project no metadata columns pay nothing. Tests cover the suppressed conflict in both validation modes, case-insensitive and case-sensitive matching, a renaming connector, and a metadata column the connector no longer reports. `InMemoryBaseTable` gains a `rename-conflicting-metadata-columns` table property, defaulting to true so existing suites are unaffected, which makes the suppressed branch reachable from a real query; `DataSourceV2DataFrameSuite` uses it to assert the error through the actual refresh path.
yyanyy
force-pushed
the
spark-dsv2-metacol-conflict-20260825
branch
from
August 26, 2026 19:49
80cd41e to
3b163ad
Compare
yyanyy
added a commit
to yyanyy/spark
that referenced
this pull request
Aug 26, 2026
…site Reverts the comment part of 8407d08. The existing one-line comment is already correct for the end state: once the companion validation lands, validation does own rejecting a suppressed metadata column. Annotating the transient window instead described a state that stops being true the moment apache#58295 lands, so it only created a follow-up edit to undo, and it changed no behavior in the meantime. The underlying question -- whether this PR is meant to land only after apache#58295, given that canRenameConflictingMetadataColumns defaults to false -- stays open for the author to answer rather than being recorded in code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
V2TableUtil.validateCapturedMetadataColumnsnow rejects the case where a data column has taken the name of a captured metadata column, instead of letting the conflict pass silently.DSv2 relations capture their metadata-column attributes at analysis time and re-validate them when the table is refreshed / re-resolved. The existing check compared them only against the metadata columns the connector still reports, so a conflict arriving on the data side was invisible.
Connectors that rename conflicting metadata columns (
SupportsMetadataColumns.canRenameConflictingMetadataColumns()istrue) keep the column reachable and are unaffected. When the conflict is suppressed instead (the default),LogicalPlan.metadataOutputWithOutConflictsdrops the metadata column, and on a partially-pruned scanPushDownUtils.toOutputAttrsresolves the read schema by name and collapses the two same-named fields onto one attribute — so a query for the metadata column silently returns the data column's values.All three callers go through the new check, but only the two that admit new data columns can fire it: refresh (
ALLOW_NEW_FIELDS) and the dataframe temp-view path (ALLOW_NEW_TOP_LEVEL_FIELDS). UnderPROHIBIT_CHANGES,V2TableReference.validateNoChangesalready throws on the added data column first, so the transactional-write path is unchanged. Only metadata columns the table still reports are considered — one the connector dropped is already flagged as removed — and the check is skipped when none remain.Related to #58298 (SPARK-59015, DSv2 refresh schema rebind), which raises an internal-error assertion for this same case that this PR's user-facing rejection makes unreachable. Either can merge first, but both now touch
InMemoryBaseTable.scalaandDataSourceV2DataFrameSuite.scala, so the later one should rebase and re-run both suites.Why are the changes needed?
Returning the wrong column's values is far worse than failing, and the
SupportsMetadataColumnscontract already recommends that non-renaming sources reject data columns that collide with metadata columns.Does this PR introduce any user-facing change?
Yes. A relation that referenced a metadata column and is later re-resolved against a table where a data column took that name now fails with "
<name>metadata column is hidden by a data column with the same name", instead of returning wrong results or working by luck depending on pruning. The condition isINCOMPATIBLE_TABLE_CHANGE_AFTER_ANALYSIS.METADATA_COLUMNS_MISMATCHon the refresh path andINCOMPATIBLE_COLUMN_CHANGES_AFTER_VIEW_WITH_PLAN_CREATION(colType=metadata) for a dataframe temp view. Connectors that rename are unaffected; this tightens a validation gap on unreleased master.How was this patch tested?
V2TableUtilSuitecovers the suppressed conflict under both validation modes, case sensitivity in both settings, a renamable connector, and a metadata column the connector stopped reporting (removed, not hidden).DataSourceV2DataFrameSuiteadds an end-to-end test where a newly added data column shadows a captured metadata column and the query now fails through the real refresh path. Reaching the suppressed branch needs a connector that does not rename, soInMemoryBaseTablegains arename-conflicting-metadata-columnsproperty defaulting totrue.catalyst/testOnly *V2TableUtilSuite54 tests,sql/testOnly *DataSourceV2DataFrameSuite204 tests, 0 failures.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code 2.1.246