[SPARK-56655][SQL] Implement remaining v2 view DDL and inspection commands#55593
Closed
cloud-fan wants to merge 18 commits intoapache:masterfrom
Closed
[SPARK-56655][SQL] Implement remaining v2 view DDL and inspection commands#55593cloud-fan wants to merge 18 commits intoapache:masterfrom
cloud-fan wants to merge 18 commits intoapache:masterfrom
Conversation
cloud-fan
force-pushed
the
v2-view-followup
branch
from
4fd6df4 to
ca821ba
Compare
…elationCatalog, loadRelation Mechanical rename follow-up to SPARK-52729: - MetadataOnlyTable -> MetadataTable - RelationCatalog -> TableViewCatalog - loadRelation() -> loadTableOrView() Test suites renamed to track the class name: - DataSourceV2MetadataOnlyTableSuite -> DataSourceV2MetadataTableSuite - DataSourceV2MetadataOnlyViewSuite -> DataSourceV2MetadataViewSuite Other v2 helpers that share the loadRelation name (CatalogV2Util.loadRelation, RelationResolution's private loadRelation(V2TableReference)) are unrelated and left as-is. Co-authored-by: Isaac
…mands
Follow-up to SPARK-52729: close out the *Remaining work* section of the
parent PR.
Write-side execs (`AlterV2ViewExec.scala`):
- ALTER VIEW ... SET TBLPROPERTIES (`AlterV2ViewSetPropertiesExec`)
- ALTER VIEW ... UNSET TBLPROPERTIES (`AlterV2ViewUnsetPropertiesExec`)
- ALTER VIEW ... WITH SCHEMA (`AlterV2ViewSchemaBindingExec`)
- ALTER VIEW ... RENAME TO (`RenameV2ViewExec`) -- backed by a new
abstract `ViewCatalog#renameView(Identifier, Identifier)`. The strategy
consults the parser-set `isView` flag and rejects ALTER TABLE syntax on
a view with EXPECT_TABLE_NOT_VIEW.USE_ALTER_VIEW (mirrors v1
`DDLUtils.verifyAlterTableType`). Cross-type rejection contract for
`renameView` is added to `TableViewCatalog`'s docs.
- A shared `V2ViewMetadataMutation.builderFrom(existing)` helper seeds a
`ViewInfo.Builder` from the existing view so mutating execs override only
the field they're changing -- schema, queryText, captured resolution
context, captured SQL configs, and queryColumnNames flow through unchanged.
Read-side execs (`V2ViewInspectionExecs.scala`):
- SHOW CREATE TABLE / SHOW TBLPROPERTIES / SHOW COLUMNS / DESCRIBE TABLE on
a v2 view -- consume the typed `ViewInfo` resolved at analysis time and
format output rows directly. DESCRIBE TABLE EXTENDED emits a v2-native
`# Detailed View Information` block.
`SHOW TABLES` on a `TableViewCatalog` now routes through
`listRelationSummaries` so views appear alongside tables, matching v1
`SHOW TABLES`. Pure `TableCatalog` catalogs continue to use `listTables` and
return tables only.
All `UNSUPPORTED_FEATURE.TABLE_OPERATION` pins from the parent PR for these
commands are replaced with real strategy cases.
Architecture notes:
- `ResolvedPersistentView` gains `viewInfo: Option[ViewInfo]`. Populated at
resolution time by the v2 paths in `Analyzer.lookupTableOrView`. v2 execs
read it directly -- no `loadView` re-fetch at exec time, matching how
v2 `ResolvedTable` carries the resolved `Table`.
Co-authored-by: Isaac
…and fold in late prod tweaks
Mirror the DROP TABLE test layout from `sql/core/test/.../command/{,v1/,v2/}` for
every v2 view DDL / inspection command added in the parent commit. Each command
lands as:
- `command/<Cmd>SuiteBase.scala` -- unified tests parameterized by `$catalog`
- `command/v1/<Cmd>Suite.scala` -- extends Base + v1 `ViewCommandSuiteBase`
(pins `$catalog` to `spark_catalog`, so the unified tests target the
session catalog)
- `command/v2/<Cmd>Suite.scala` -- extends Base + v2 `ViewCommandSuiteBase`
(pins `$catalog` to a fresh `test_view_catalog` backed by a new
general-purpose `InMemoryTableViewCatalog` test fixture), plus
catalog-state assertions specific to the v2 fixture
Triplets cover: CREATE VIEW, ALTER VIEW ... AS, ALTER VIEW SET / UNSET
TBLPROPERTIES, ALTER VIEW RENAME TO, ALTER VIEW WITH SCHEMA, SHOW CREATE
TABLE, SHOW TBLPROPERTIES, SHOW COLUMNS, DESCRIBE TABLE, DESCRIBE TABLE
... COLUMN, DROP VIEW, SHOW VIEWS.
Each Base test runs against both `spark_catalog` (v1, hits the existing v1
commands) and `test_view_catalog` (v2, hits the new execs from the parent
commit), giving a single source of cross-catalog behavioral parity.
Supporting infrastructure:
- `sql/catalyst/test/.../connector/catalog/InMemoryTableViewCatalog.scala`:
a general-purpose `TableViewCatalog` for tests, with shared table/view
keyspace and a runtime-type kind discriminator.
- `command/v1/ViewCommandSuiteBase.scala` and
`command/v2/ViewCommandSuiteBase.scala`: extend the existing v1/v2
`CommandSuiteBase` so view tests inherit `checkLocation` etc.
Folded in here as well (chronologically these landed alongside the test
work and depend on the test infrastructure to exercise):
- Reject `ALTER TABLE <view> RENAME TO ...` with EXPECT_TABLE_NOT_VIEW
.USE_ALTER_VIEW. The parser routes both `ALTER TABLE ... RENAME TO`
and `ALTER VIEW ... RENAME TO` to the same `RenameTable` plan with an
`isView` flag; the v2 strategy now consults the flag and rejects the
wrong-syntax case (mirrors v1 `DDLUtils.verifyAlterTableType`).
- DESCRIBE TABLE ... COLUMN on a v2 view: surface the view's schema as
`ResolvedPersistentView.output` (with char/varchar normalization).
`ResolveReferences` resolves the column reference against it -- analogous
to how `ResolvedTable.output` lets the column resolve naturally for
tables. `CheckAnalysis` no longer trips, the plan stays as
`DescribeColumn(ResolvedPersistentView, ...)` until strategy time, and
the v2 case extracts `nameParts` from the resolved attribute and
dispatches to a new `DescribeV2ViewColumnExec`. The v1 rewrite in
`ResolveSessionCatalog` accepts both the resolved `Attribute` form and
the legacy `UnresolvedAttribute` form.
The pre-existing `DataSourceV2MetadataViewSuite` is trimmed: CREATE / ALTER /
DROP / SHOW VIEW DDL coverage moves to the per-catalog triplets. What remains
in the leaf suite is genuinely v2-specific structural coverage (view read-path,
V1Table.toCatalogTable round-trip, pure-ViewCatalog read + ALTER, multi-level-
namespace cyclic detection / error rendering, REFRESH / ANALYZE rejection,
SHOW TABLES on a TableViewCatalog returning both kinds).
220 view-related tests pass locally across 29 suites.
Co-authored-by: Isaac
cloud-fan
force-pushed
the
v2-view-followup
branch
from
ca821ba to
c121e07
Compare
Fixes from self-review of the v2 view DDL/inspection follow-up: - V1ViewInfo.builderFrom now bridges CatalogTable.collation/comment into the ViewInfo properties bag under PROP_COLLATION/PROP_COMMENT, so ApplyDefaultCollation.fetchDefaultCollation (which reads from properties) picks up the existing collation for v1 ALTER VIEW AS instead of returning None and skipping literal transformation. - DescribeRelationJsonCommand pattern-matches `info: V1ViewInfo` instead of unconditionally casting; non-session v2 views now fall through to the catch-all that surfaces describeAsJsonNotSupportedForV2TablesError, matching the v2 table path, instead of throwing ClassCastException. - ShowV2ViewPropertiesExec / DescribeV2ViewExec / ShowCreateV2ViewExec filter CatalogV2Util.TABLE_RESERVED_PROPERTIES (PROP_COMMENT, PROP_COLLATION, PROP_OWNER, PROP_TABLE_TYPE, ...) instead of just PROP_TABLE_TYPE, so SHOW TBLPROPERTIES / DESCRIBE EXTENDED on a v2 view no longer leak first-class fields that v1 hides. - AlterV2ViewSetPropertiesExec / UnsetPropertiesExec call CommandUtils.uncacheTableOrView before replaceView, matching v1 AlterTableSetPropertiesCommand's invalidateCachedTable. - Rewrite stale comments: ResolveSessionCatalog.ResolvedViewIdentifier, V2ViewInspectionExecs and DataSourceV2Strategy referenced `ResolvedPersistentView.viewInfo` and "tracked for a follow-up PR" text that no longer applied; minor grammar in DataSourceV2MetadataViewSuite. Co-authored-by: Isaac
…R VIEW AS Address Pass-A finding from the v2 view DDL/inspection self-review: when an existing view has no `PROP_COLLATION` and its namespace has a default collation, `ALTER VIEW AS` now folds the namespace default into the persisted view metadata, on both v1 and v2 paths. Three pieces: 1. v1: plumb the analysis-time collation through `AlterViewAsCommand`. `ResolveSessionCatalog` reads `ResolvedPersistentView.info.properties`'s `PROP_COLLATION` (post-`ApplyDefaultCollation` rewrite) and passes it to `AlterViewAsCommand`. `alterPermanentView` writes it onto the persisted `CatalogTable.collation` so subsequent reads pick it up via `AnalysisContext.collation`. Without this fix, the analyzed plan's literals were collated correctly but the persisted typed `collation` field was lost because `alterPermanentView` reloaded `viewMeta` from the catalog and used `viewMeta.copy(...)` without touching `collation`. 2. v2: generalize `ApplyDefaultCollation.resolveDefaultCollation`'s `AlterViewAs` case to fire for any `ResolvedPersistentView` whose `info.properties.PROP_COLLATION` is empty -- both `V1ViewInfo` (existing v1 path) and plain `ViewInfo` (v2 path). The v2 branch rebuilds the `ViewInfo` with the namespace default via a shared `CatalogV2Util.viewInfoBuilderFrom` helper (extracted from the existing `V2ViewMetadataMutation.builderFrom` and reused). Gate the rewrite on a non-empty namespace default to avoid an infinite resolution loop -- `ViewInfo` / `V1ViewInfo` are not case classes, so a copy with identical field values reads as a different reference under fast-equals. 3. Tests: extend `InMemoryTableViewCatalog` with a minimal `SupportsNamespaces` impl so v2 view tests can exercise namespace metadata; add a v1 scenario-C test in `DefaultCollationTestSuite` and a matching v2 test in `command/v2/AlterViewAsSuite`. Co-authored-by: Isaac
…doc V1ViewInfo properties caveat, tighten tests - RenameV2ViewExec now captures the cached storage level before uncaching and re-instates the cache at the new identifier afterwards, mirroring v1 AlterTableRenameCommand and v2 RenameTableExec. Without it a user-cached view on a v2 catalog silently lost its cache after RENAME. - V1ViewInfo Scaladoc calls out that the inherited properties() bag mixes user TBLPROPERTIES with v1-internal storage keys (view.sqlConfig.*, view.catalogAndNamespace.*, view.query.out.*, view.schemaMode), and consumers should prefer typed accessors for the v1-internal fields. - ShowCreateViewSuiteBase "renders the column list" test now uses distinctive column names so the contains() assertions aren't satisfied by tokens elsewhere in the rendered DDL. - AlterViewRenameSuiteBase adds a "rename re-caches a previously cached view" base test that runs against both v1 and v2 paths and pins the new RenameV2ViewExec cache-restore behavior. - AlterViewRenameSuite (v2 leaf) adds a cross-namespace rename test exercising RenameV2ViewExec's non-empty-namespace branch.
…tadata in ApplyDefaultCollation; pin SET TBLPROPERTIES('comment') round-trip through SHOW CREATE TABLE
- ApplyDefaultCollation's AlterViewAs case used to call `getCollationFromSchemaMetadata`
twice (once in the guard, once in the body), issuing two `loadNamespaceMetadata` round
trips per resolution-batch iteration. Restructure to a single lookup and a
match-on-Option so the connector sees one round trip.
- AlterViewSetTblPropertiesSuiteBase: add a unified test that pins SET TBLPROPERTIES
('comment' = ...) flowing through to SHOW CREATE TABLE on both v1 and v2 paths --
v1 updates the typed `CatalogTable.comment` field, v2 stores via `ViewInfo.properties`,
but the user-visible behavior must match.
Co-authored-by: Isaac
… into Base Both `CREATE VIEW over a non-view table entry` and `DROP VIEW on a non-view table entry` were v2-only tests despite asserting pure SQL behavior. Move the SQL-behavior assertions into the shared *SuiteBase, with a small abstract hook `withSeededTable(qualified)(body)` that each leaf implements via SQL `CREATE TABLE ... USING parquet`. Net effect: v1 also exercises this parity now, and the v2 leaves keep only their genuinely v2-specific post-conditions (catalog-state poking via `getStoredInfo` / `tableExists`). Surfaced one pre-existing v1/v2 divergence in the lift: v1 `DropTableCommand` raises `WRONG_COMMAND_FOR_OBJECT_TYPE` for `DROP VIEW` on a table while v2 `DropViewExec` raises `EXPECT_VIEW_NOT_TABLE`. The lifted Base test accepts either condition with an inline comment; aligning the two error classes is out of scope. Co-authored-by: Isaac
…COMMAND_FOR_OBJECT_TYPE)
The v2 view DDL surface introduced in SPARK-52729 + SPARK-56655 is unreleased,
so we can clean up the error class for the two `DROP <wrong-kind>` cases
without a user-facing break.
Changes:
- `DropViewExec` (DROP VIEW on a table) and `DropTableExec` (DROP TABLE on a
v2 view) switch from `EXPECT_VIEW_NOT_TABLE.NO_ALTERNATIVE` /
`EXPECT_TABLE_NOT_VIEW.NO_ALTERNATIVE` to `WRONG_COMMAND_FOR_OBJECT_TYPE`.
v1's `DropTableCommand` already uses `WRONG_COMMAND_FOR_OBJECT_TYPE` for the
same cases, so this aligns v2 to v1 and gives users the actionable hint
("Use DROP TABLE instead." / "Use DROP VIEW instead.") that the
`.NO_ALTERNATIVE` subclass omits.
Other v2 view DDL "wrong-kind" sites are intentionally left alone:
* `EXPECT_*_NOT_*.USE_ALTER_*` already conveys the alternative through its
subclass (no information loss to switch).
* CREATE OR REPLACE VIEW on a non-view table goes through a helper shared
with v1 (released) and has no clean alternative command to suggest.
- Tighten the lifted Base test in `DropViewSuiteBase` to assert the now-
uniform `WRONG_COMMAND_FOR_OBJECT_TYPE` condition + the rendered "Use DROP
TABLE instead" hint, instead of the earlier compromise that accepted either
error class.
- Inline `withSeededTable` into `CreateViewSuiteBase` / `DropViewSuiteBase`.
Both v1 and v2 leaves were implementing it identically (SQL `CREATE TABLE
... USING parquet`) -- `InMemoryTableViewCatalog.createTable` accepts the
parquet `TableInfo` the same way the session catalog does -- so the abstract
hook was carrying no per-leaf customization. Drop the leaf overrides.
Co-authored-by: Isaac
…urceV2Strategy Restore the v2 strategy match for CreateTableLike when the source is a non-session v2 view: previously the case narrowed to V1ViewInfo only, so a v2 ViewInfo source fell through to a runtime MatchError. Synthesize the CatalogTable via V1Table.toCatalogTable for non-V1ViewInfo views, mirroring the pre-narrowing behavior. Co-authored-by: Isaac
…view; restore DESC AS JSON for non-session v2 views - ResolveSessionCatalog: rewrite `DESCRIBE TABLE <view> PARTITION (...)` on a non-session v2 view (where the existing v1 rewrite is gated on `ResolvedV1TableOrViewIdentifier`) to throw `descPartitionNotAllowedOnView` early. Without this, `UnresolvedPartitionSpec` is never resolved on the v2 view path and CheckAnalysis surfaces a generic "Found the unresolved operator" INTERNAL_ERROR. Mirrors the v1 runtime check in `DescribeTableCommand.describeDetailedPartitionInfo`. - DescribeRelationJsonCommand: restore pre-PR behavior of `DESC ... AS JSON` on non-session v2 views. The first cut narrowed the view branch to `V1ViewInfo` only, so non-session v2 views fell through to `describeAsJsonNotSupportedForV2TablesError` -- a regression vs. master (where the synthesized `CatalogTable` made the JSON path uniform). Restore the uniform behavior by recovering the v1 `CatalogTable` from `V1ViewInfo` and synthesizing an equivalent via `V1Table.toCatalogTable` for v2 views, mirroring the `CreateTableLike` strategy case. - New tests in `DataSourceV2MetadataViewSuite` for both fixes. Co-authored-by: Isaac
…; DESCRIBE EXTENDED first-class fields; shared DescribeColumn helper; clear queryColumnNames on EVOLUTION
Four self-review findings rolled into one commit:
1. SHOW COLUMNS namespace conflict on v2 view (Pass A finding 1).
The v2 strategy case `case ShowColumns(rpv @ ResolvedPersistentView(_, _, _),
_, output)` discarded the explicit `FROM <ns>` argument; v1 validates that
the view's resolved namespace matches and errors with
`SHOW_COLUMNS_WITH_CONFLICT_NAMESPACE`. Add the same cross-check to the v2
case (multi-part-namespace-aware via `Identifier.namespace().toSeq`) and a
covering test in `ShowViewColumnsSuiteBase` so both v1 and v2 legs assert
the same error condition.
2. DESCRIBE TABLE EXTENDED on v2 view promotes first-class fields (Pass A
finding 2). v1 `CatalogTable.toJsonLinkedHashMap` renders Owner / Comment /
Collation as their own rows above the generic Table Properties block; the
first cut of `DescribeV2ViewExec` collapsed all three into the Properties
string. Promote them to dedicated rows (matching the v1 row order: Owner
first, then Comment, then Collation, before the View Text block) so users
on a v2 catalog see the same shape as on the session catalog. New test in
`DescribeViewSuiteBase` covers Comment + Collation on both legs.
3. Shared DescribeColumn name-parts helper (Pass A finding 3). The
`column match { UnresolvedAttribute / Attribute / Alias / _ }` block was
duplicated verbatim in `ResolveSessionCatalog` (v1 view rewrite) and
`DataSourceV2Strategy` (v2 view case) -- both paths added by this PR.
Extract to `DescribeColumn.extractColumnNameParts` on the companion object
in `v2Commands.scala`; both call sites now go through it. Clean up the now-
unused `Alias` and `UnresolvedAttribute` imports in the strategy file.
4. Clear queryColumnNames when ALTER VIEW switches to EVOLUTION (Pass B
finding 1). v1 `generateViewProperties` empties the view-query-output
column list when `viewSchemaMode == SchemaEvolution` because EVOLUTION
always uses the view's current schema as the column source; the new
`AlterV2ViewSchemaBindingExec` was preserving the existing array via
`viewInfoBuilderFrom(existing).withSchemaMode(...)`, leaving non-canonical
metadata in the catalog. Add the explicit `withQueryColumnNames(empty)`
for the EVOLUTION branch and a test in `v2.AlterViewSchemaBindingSuite`
that pins the cleared-then-restored shape.
Co-authored-by: Isaac
…o Base Two v2-only tests in the per-command triplets were asserting pure SQL behavior that v1 hits identically. Move them into the shared *SuiteBase so both v1 and v2 catalogs exercise the same assertions, leaving each v2 leaf with only genuinely v2-specific tests (catalog state poking, v2-only error scenarios, v2-native output formats): - `ShowViewsSuiteBase` gains "does not include non-view table entries". v1 routes `SHOW VIEWS` through the session catalog with a views-only filter; v2 routes it through `ViewCatalog.listViews`. Both must exclude tables that share the same namespace and mark `isTemporary` as false. The seed table is created via SQL `CREATE TABLE ... USING parquet`, accepted uniformly by the session catalog and `InMemoryTableViewCatalog`. - `DescribeViewSuiteBase` gains "describe extended includes Catalog and View Text rows". Both v1 `DescribeTableCommand` and v2 `DescribeV2ViewExec` emit these rows in the EXTENDED block; pinning them in the Base catches any future regression on either path. The v2 leaf retains only the `# Detailed View Information` header test -- v1 emits `# Detailed Table Information` for views, so this assertion is genuinely v2-specific and shouldn't be lifted. Co-authored-by: Isaac
Several comments in this PR's added code described what the code does in terms of "pre-PR behavior", "before the X change", "Post-migration", etc. That phrasing relies on the reader having the PR's history loaded as context; once the PR merges, a future maintainer reading the same comment has no way to recover the prior state being referenced. Rewrite each affected comment to describe what the code does *now*, citing the structural reason rather than the historical change: - `DescribeRelationJsonCommand.scala`: "preserves pre-PR behavior" -> "the command projects `ViewInfo` to a `CatalogTable` via `V1Table.toCatalogTable` so DESC ... AS JSON works uniformly across session and non-session view catalogs." - `views.scala` (`AlterViewAsCommand.collation`): "callers that omit this argument retain the pre-PR behavior" -> "callers that omit this argument keep the existing view's collation untouched." - `DataSourceV2MetadataViewSuite.scala` (4 comments): "Before the `fullIdent` change ...", "Before the error signatures took `Seq[String]` ...", "Pre-PR, `ResolvedPersistentView.metadata: CatalogTable` was always populated ...", "Post-migration the errors render the full multi-part name." -> all rewritten to describe the current routing (via `fullIdent` / `Seq[String]` / `V1Table.toCatalogTable`) and why it preserves multi-level-namespace segments in user-visible messages. No code changes -- comment-only. Co-authored-by: Isaac
…iew; regen 9 analyzer-test goldens
Two CI failures, both originating in this PR's `ResolvedPersistentView.metadata: CatalogTable`
-> `ResolvedPersistentView.info: ViewInfo` migration:
1. `PlanResolutionSuite "alter view: alter view properties"` NPE. The Mockito mock for the
"view" identifier in `createV1TableMock` did not stub the view-only fields. Mockito
defaults unstubbed Object returns to `null`, so when v1 resolution wraps the mock in
`V1ViewInfo(v1Table)` and `V1ViewInfo.builderFrom` calls `v1Table.viewText.getOrElse("")`,
the call site receives a literal Java `null` (not `None`) and NPEs. Stub `viewText`,
`viewCatalogAndNamespace`, `viewSQLConfigs`, `viewQueryColumnNames` for the VIEW case so
the mock matches the shape a real `CatalogTable` would have. Real `CatalogTable`s never
produce null Options because their case-class defaults are `None` / `Seq.empty` / etc., so
no production-side defensive code is needed.
2. 9 `SQLQueryTestSuite *_analyzer_test` golden file mismatches. The case-class default
`toString` rendered the new `info: ViewInfo` field via `Object.toString` -- producing
`V1ViewInfo@<hash>` (and a similarly opaque hash for v2 plain-`ViewInfo`) -- non-deterministic
between JVM runs, which is exactly what golden files cannot tolerate. Override
`ResolvedPersistentView.stringArgs` so the third positional element renders as the
qualified `catalog.namespace.name` string (via `quoted`), matching the spirit of the
pre-migration `CatalogTable.toString` output but stable under hash randomization.
Regenerate the 9 affected analyzer-test goldens via `SPARK_GENERATE_GOLDEN_FILES=1`; the
only delta vs. master is the new stable rendering plus, for ALTER VIEW AS goldens, a
trailing `<collation>` argument from the new `AlterViewAsCommand.collation` field added
earlier in this PR.
Co-authored-by: Isaac
…g state The previous regen in 19b2d5b captured fresh-catalog plan outputs for 4 queries that, under SQLQueryTestSuite's shared warehouse, actually resolve to *_ALREADY_EXISTS errors when the analyzer-test runs after the regular test. Re-regenerate those goldens with both tests in scope so the captured outputs match CI behavior; the legitimate ResolvedPersistentView rendering changes from this PR are preserved. Co-authored-by: Isaac
gengliangwang
approved these changes
Apr 30, 2026
Member
|
Thanks, merging to master |
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
2 participants
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?
Follow-up to SPARK-52729 (which added
MetadataOnlyTableand CREATE / ALTER VIEW … AS support for DS v2 catalogs). This PR closes out the Remaining work section of that PR's description, plus a few API/test cleanups.The branch is structured as three commits:
1.
[SPARK-56655][SQL] Rename v2 metadata-table API: MetadataOnlyTable, RelationCatalog, loadRelation— mechanical rename of the v2 view-API surface introduced by the parent PR:MetadataOnlyTableMetadataTableRelationCatalogTableViewCatalogloadRelation()loadTableOrView()DataSourceV2MetadataOnly{Table,View}SuiteDataSourceV2Metadata{Table,View}SuiteThe unrelated v2 helpers
CatalogV2Util.loadRelationandRelationResolution's privateloadRelation(V2TableReference)predate the parent PR and are intentionally not renamed.2.
[SPARK-56655][SQL] Implement remaining v2 view DDL and inspection commands— the production-side work:New view DDL execs (
AlterV2ViewExec.scala):AlterV2ViewSetPropertiesExec,AlterV2ViewUnsetPropertiesExec— merge / drop user TBLPROPERTIES on the existing view; dispatch toViewCatalog#replaceView.AlterV2ViewSchemaBindingExec— rewrites the schema-binding mode field; dispatch toreplaceView.RenameV2ViewExec— dispatches to a new abstractViewCatalog#renameView(Identifier, Identifier)(added toViewCatalog, contracted onTableViewCatalog).V2ViewMetadataMutation.builderFrom(existing)helper seeds aViewInfo.Builderfrom the existing view so mutating execs override only the field they're changing.New view inspection execs (
V2ViewInspectionExecs.scala):ShowCreateV2ViewExec— reconstructs theCREATE VIEW …DDL fromViewInfo.ShowV2ViewPropertiesExec,ShowV2ViewColumnsExec,DescribeV2ViewExec,DescribeV2ViewColumnExec— produce output rows directly fromViewInfo.DESCRIBE TABLE EXTENDEDemits a v2-native# Detailed View Informationblock.v1-parity for
SHOW TABLESon aTableViewCatalog(ShowTablesExec.scala): when the catalog is aTableViewCatalog, route throughlistRelationSummariesso views appear alongside tables. PureTableCatalogcatalogs continue to uselistTablesand return tables only.All
UNSUPPORTED_FEATURE.TABLE_OPERATIONpins from the parent PR for these commands are replaced with real strategy cases.Architecture — single typed payload for resolved views. To match the v2-table convention (where
ResolvedTable.table: Tablecarries the resolved Table — and v1 tables appear asV1Tablewrapping aCatalogTable),ResolvedPersistentViewnow carriesinfo: ViewInfo:A new
private[sql] V1ViewInfo(v1Table: CatalogTable) extends ViewInfoexposes a v1CatalogTablethrough the v2ViewInfosurface, mirroringV1TableforTable.ViewInfo's constructor relaxed fromprivatetoprotectedso the subclass can call it. v1-only paths (DescribeTableCommand viaResolvedChildHelper, the v1 DescribeRelation JSON path, ApplyDefaultCollation's AlterViewAs rewrite, CreateTableLike strategy case) recover the originalCatalogTableviainfo.asInstanceOf[V1ViewInfo].v1Table. v2 strategy cases consumerpv.infodirectly.ResolvedPersistentView.outputnow exposes the view's schema attributes (with char/varchar normalization), soDescribeColumnagainst a v2 view survivesCheckAnalysis— the column resolves naturally throughResolveReferences, and the strategy / v1 rewrite both extractnamePartsfrom the resolved attribute. No new logical plan needed for DESCRIBE COLUMN; the existing one flows to the planner intact.Also folded into this commit:
ALTER TABLE <view> RENAME TO …is rejected withEXPECT_TABLE_NOT_VIEW.USE_ALTER_VIEWon the v2 path (mirrors v1DDLUtils.verifyAlterTableType).3.
[SPARK-56655][SQL][TESTS] Add per-catalog view command test triplets and fold in late prod tweaks— test scaffolding:Mirror the DROP TABLE test layout from
sql/core/test/.../command/{,v1/,v2/}for every v2 view DDL / inspection command. Each command lands as:command/<Cmd>SuiteBase.scala— unified tests parameterized by$catalogcommand/v1/<Cmd>Suite.scala— extends Base + v1ViewCommandSuiteBase(pins$catalogtospark_catalog, so the unified tests target the session catalog)command/v2/<Cmd>Suite.scala— extends Base + v2ViewCommandSuiteBase(pins$catalogto a freshtest_view_catalogbacked by a new general-purposeInMemoryTableViewCatalogtest fixture), plus catalog-state assertions specific to the v2 fixtureTriplets cover: CREATE VIEW, ALTER VIEW … AS, ALTER VIEW SET / UNSET TBLPROPERTIES, ALTER VIEW RENAME TO, ALTER VIEW WITH SCHEMA, SHOW CREATE TABLE, SHOW TBLPROPERTIES, SHOW COLUMNS, DESCRIBE TABLE, DESCRIBE TABLE … COLUMN, DROP VIEW, SHOW VIEWS.
Each Base test runs against both
spark_catalog(v1, hits the existing v1 commands) andtest_view_catalog(v2, hits the new execs from commit #2), giving a single source of cross-catalog behavioral parity.The pre-existing
DataSourceV2MetadataViewSuiteis trimmed: CREATE / ALTER / DROP / SHOW VIEW DDL coverage moves to the per-catalog triplets. What remains in the leaf suite is genuinely v2-specific structural coverage (view read-path, V1Table.toCatalogTable round-trip, pure-ViewCatalog read + ALTER, multi-level-namespace cyclic detection / error rendering, REFRESH / ANALYZE rejection, SHOW TABLES on aTableViewCatalogreturning both kinds).Why are the changes needed?
The parent PR shipped CREATE VIEW + ALTER VIEW … AS through the v2 surface but pinned the rest of the view DDL/inspection family with
UNSUPPORTED_FEATURE.TABLE_OPERATION. Third-party v2 catalogs that host views still couldn't runALTER VIEW SET TBLPROPERTIES,ALTER VIEW … RENAME TO,ALTER VIEW … WITH SCHEMA <mode>,DESCRIBE,SHOW CREATE TABLE,SHOW TBLPROPERTIES,SHOW COLUMNSagainst their views — full v1 parity for non-session view catalogs requires this set.The rename to
MetadataTable/TableViewCatalog/loadTableOrViewwas discussed during the parent PR's review as a clarity improvement; doing it now (before the API ships in a release) avoids deprecation churn later.ResolvedPersistentView.info: ViewInfo(withV1ViewInfofor v1 views) brings v2 view command flow in line with the existing v2 table command convention (ResolvedTable.table: Table, withV1Tablefor v1 tables) — single typed payload, resolved at analysis time, consumed at exec time. Looking up database objects at runtime is the anti-pattern this removes.Does this PR introduce any user-facing change?
Yes for connector developers:
@Evolvingv2 view API surface. Connectors implementing the parent PR'sRelationCatalog/ overridingloadRelation/ referencingMetadataOnlyTableneed to update toTableViewCatalog/loadTableOrView/MetadataTable.ViewCataloggains a new abstractrenameView(oldIdent, newIdent)method. ExistingViewCatalogimplementations need to add it (the parent PR has not yet released, so this is still pre-release breakage).ViewInfo's constructor is nowprotected(wasprivate). Existing call sites use theViewInfo.Builder; only internal subclasses need to call the constructor directly.Yes for end users on a non-session v2 view catalog: the listed DDL / inspection commands now succeed against a
ViewCataloginstead of erroring withUNSUPPORTED_FEATURE.TABLE_OPERATION.SHOW TABLESon aTableViewCatalognow returns both tables and views, matching v1 SHOW TABLES output.ALTER TABLE <view> RENAME TO …(wrong syntax) now returnsEXPECT_TABLE_NOT_VIEW.USE_ALTER_VIEWinstead of silently succeeding.No user-facing change on the session catalog path — those plans are still rewritten to the v1 commands by
ResolveSessionCatalogand behave exactly as before.How was this patch tested?
*SuiteBasetriplets undersql/core/src/test/.../command/{,v1/,v2/}exercise each command against both a v1 (session) catalog and a fresh v2InMemoryTableViewCatalogfixture.DataSourceV2MetadataViewSuiteis trimmed to v2-specific structural tests (read path,V1Table.toCatalogTableround-trip, pure-ViewCatalog read+ALTER, multi-level-namespace cyclic / error rendering, REFRESH/ANALYZE rejection, SHOW TABLES TableViewCatalog).ResolvedChildHelper.getTableMetadataafter the metadata→info refactor).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude (Anthropic)