[SPARK-56643][CONNECT][TESTS] Add DSv2 temp view with stored plan tests using Spark Connect mode

[longvu-db]

What changes were proposed in this pull request?

Extract the 21 DSv2 temp view with stored plan tests from DataSourceV2DataFrameSuite into a shared trait DSv2TempViewWithStoredPlanTests, and add a Connect-mode runner DataSourceV2TempViewConnectSuite so the same tests run in both classic and Connect modes.

The shared trait covers these scenarios:

1. Session and external writes (scenarios 1.1, 1.2, 1.2+cache): Temp view with filter reflects new data.
2. Adding columns (scenarios 2.1, 2.2, 2.2+cache): Temp view preserves original schema after ADD COLUMN.
3. Removing columns (scenarios 3.1, 3.2, 3.2+cache): Temp view detects column removal.
4. Drop and recreate table (scenarios 4.1, 4.2, 4.2+cache): Temp view resolves to recreated table.
5. Drop and re-add column with same type (scenarios 5.1, 5.2, 5.2+cache): Schema validation passes, view continues working with null values.
6. Drop and re-add column with different type (scenarios 6.1, 6.2, 6.2+cache): Temp view detects type change.
7. Type widening (scenarios 7.1, 7.2, 7.2+cache): Temp view detects INT to BIGINT type change.

Each test creates a DSv2 table, inserts initial data, builds a temp view with a filter (salary < 999) to demonstrate stored plan behavior, and verifies the view after table modifications. External writes use the direct catalog API (loadTable + withData) to simulate writes from outside the session. Caching connector variants use CachingInMemoryTableCatalog to verify stale-cache behavior.

This is the Spark Connect counterpart of #55540.

Refactoring

• DSv2ExternalMutationTestBase (new): Abstract base trait with session-agnostic hooks (withTestSession, checkRows, getTableCatalog, withTestTableAndViews, externalAppend) so the same test logic runs in both classic and Connect modes.
• DSv2TempViewWithStoredPlanTests (new): Shared trait containing the 21 test methods, extracted from DataSourceV2DataFrameSuite.
• DataSourceV2TempViewConnectSuite (new): Connect-mode runner that provides Connect-specific session, catalog access, and result comparison.
• DataSourceV2DataFrameSuite (modified): Mixes in DSv2TempViewWithStoredPlanTests instead of inlining the tests; adds classic-mode implementations of the abstract hooks.

Why are the changes needed?

The existing temp view tests in DataSourceV2DataFrameSuite use classic-only APIs (checkAnswer, catalog() helper) and cannot run in Connect mode. Extracting them into a shared trait enables Connect parity testing.

Does this PR introduce any user-facing change?

No. This PR only adds tests and refactors test infrastructure.

How was this patch tested?

All 21 shared tests pass in both modes:

build/sbt 'sql/testOnly *DataSourceV2DataFrameSuite'
build/sbt 'connect/testOnly *DataSourceV2TempViewConnectSuite'

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (claude-opus-4-6)

[andreaschat-db]

Thanks @longvu-db. I left a few comments.

[andreaschat-db]

LGTM.

[longvu-db]

Auto-review results

1 critical issue found and fixed:

CachingInMemoryTableCatalog.clearCache() in withTableAndViews (line 87) was called as a companion-object static method, but no companion object exists — only an instance method. This would not compile.

Fix (commit 0b76175): Get the server-side catalog instance and call clearCache() on it, matching the pattern used by the classic DataSourceV2DataFrameSuite:

if (clearCachingCatalog) {
  val serverSession = getServerSession(session)
  serverCatalog[CachingInMemoryTableCatalog](serverSession, "cachingcat").clearCache()
}

Also removed CachingInMemoryTableCatalog.scala and InMemoryTableCatalog.scala from this PR (both already in master).

No other issues found. Test logic, assertions, error conditions, cleanup patterns, imports, and style all look good. ✅

[cloud-fan]

Test-only PR that adds DataSourceV2TempViewConnectSuite with 21 Connect-mode parity tests mirroring the classic DataSourceV2DataFrameSuite "temp view with stored plan" scenarios. Architecture is reasonable for this kind of parity coverage: in-process Connect server via SparkConnectServerTest, client-driven reads/writes through withSession, server-side catalog peeks via getServerSession + serverCatalog[T] for external mutations. Most of @andreaschat-db's earlier nits are addressed; a few helper-design questions remain.

PR description vs. diff

Two pieces of the PR description no longer match what's in the diff:

1. "Add 11 new tests" — the file actually adds 21 tests (each scenario has session, external, and cached-connector variants). The opportunistic extra coverage is welcome, just worth updating the count.
2. The "Shared test infrastructure changes" block describes a new CachingInMemoryTableCatalog and a BasicInMemoryTableCatalog.alterTable fix, but per commits 5c64c989f8e ("Remove ... already in master") and 58135982455 ("Restore catalog files to master version") neither is in this PR anymore. Trim that section before merge.

Duplication with the analogue

All 21 tests are line-for-line parity copies of tests already in DataSourceV2DataFrameSuite (lines 3009–3568) — no shared trait, just sql(...)/withTable/checkAnswer/catalog(name) swapped for session.sql(...).collect()/withTableAndViews/assertRows/serverCatalog[T]. Was a shared trait considered? The analogue extending InsertIntoTests makes factoring non-trivial so I understand if the answer is "duplication is the lesser evil," but a one-line acknowledgement in the PR description would help future scenario additions remember to update both files.

Process note

@andreaschat-db's approval predates five commits (the latest being the 2026-05-20 auto-review fix). A re-confirm seems appropriate before merge.

[longvu-db]

CI failed with unrelated with unrelated flaky RocksDBStateStoreIntegrationSuiteWithRowChecksum failure

[cloud-fan]

The shared-trait refactor since the last round (extracting 21 tests into DSv2TempViewWithStoredPlanTests over a DSv2ExternalMutationTestBase hook surface) addresses the duplication concern from the prior round, and the Connect-side justifications for QueryTest.sameRows (DataSourceV2TempViewConnectSuite.scala:46-53) and dropping clearCache() (:69-72) are accurate — I verified both. Correction to my prior round: I claimed checkAnswer is usable from Connect server tests; that's true only for classic server-side DataFrames, not for the Connect-client DataFrames this suite uses (QueryTest.checkAnswer accesses queryExecution/logicalPlan/materializedRdd, which throw on Connect client DataFrames). Your sameRows switch is correct; ignore the earlier suggestion.

Status against the prior AI review: 3 addressed, 0 remaining, 2 new. The prior commit (0b76175) is no longer an ancestor of HEAD (rebased), so I'm treating new findings uniformly.

Bigger design question — coordination with #55356

@fwc's PR #55356 ("SharedSparkSession provides sql.SparkSession, add SharedClassicSparkSession, ClassicQueryTest") is pursuing the same goal of sharing tests between classic and Connect, but at the foundation layer: it lifts SharedSparkSession.spark to sql.SparkSession, moves classic-only helpers into classic.SharedSparkSession/classic.QueryTest subtraits, and makes QueryTest itself extendable in a Connect-providing suite.

If that direction lands, three of the four hooks in DSv2ExternalMutationTestBase become unnecessary:

• withTestSession(fn) — because spark itself is the abstract sql.SparkSession.
• checkRows — if checkAnswer is made Connect-aware in shared infra, instead of every shared trait re-implementing it.
• withTestTableAndViews — once withTable operates on sql.SparkSession.

Only getTableCatalog would remain, because the catalog-mutation bypass is genuinely Connect-specific.

Merge order doesn't matter, but the direction needs to be one direction, not two. Could you sync with @fwc on whether the right path is:

(a) wait for #55356 to land and rebase this PR onto the lifted SharedSparkSession, dropping the redundant hooks; or

(b) merge this PR first but contribute the generic Connect primitives (checkAnswer dispatch, withTable lift) into shared infrastructure as part of #55356, so future shared traits don't reinvent them.

What would be unfortunate is leaving a private 4-hook abstraction in this trait and a parallel foundational abstraction in #55356 — that's two test-sharing frameworks to reconcile later. The shared-trait pattern in StaticProcedureSuiteBase / sql.SparkSessionBuilderImplementationBindingSuite (plain extends AnyFunSuite, no per-trait routing) is what this should look like once the foundation is in place.

Process note

@andreaschat-db's LGTM (2026-05-08, commit ae21481) predates the entire shared-trait refactor (introduced in b879d2c). A re-confirm seems appropriate before merge.

Two smaller inline findings below.