Add ConnectionFactory + connection-state/transaction negative suites (PR 5a/8)#1528
Merged
Merged
Conversation
…(PR 5a/8) Introduces the fresh-connection mechanism for negative suites whose cases mutate or destroy connection/session state, plus the first two such suites. (Splits the planned PR 5 into 5a here + 5b later — CONNECTION, CANCEL_TIMEOUT, VOLUME — so the count is now 8.) Mechanism: - config/ConnectionFactory.openFresh(side) — a dedicated, uncached connection - ConnectionManager.openUncached(url) — DriverManager.getConnection, not cached - SuiteProvider: backward-compatible default 5-arg execute(conn1, conn2, ConnectionFactory, tc, label) delegating to the 4-arg form; only isolation suites override it, the 12 existing providers are untouched - JDBCDriverComparisonTest builds a per-config factory bound to the resolved LEFT/RIGHT URLs and calls the 5-arg form Suites (each opens its own fresh connections and closes them in a finally, so they never poison the shared connections): - NEGATIVE_CONNECTION_STATE: setCatalog/setSchema/setClientInfo/USE to a nonexistent target - NEGATIVE_TRANSACTION: commit/rollback with autocommit on; DDL inside a manual transaction (throwaway table dropped in a finally) Also fixes one-sided-diff mis-filing exposed by these suites: ErrorDiffs.compare returns a FLAT list, so callers that dumped it into dataDifferences mis-filed one-sided diffs (which belong in metadataDifferences) and dropped them from the CSV summary. New ErrorDiffs.foldInto(result, ...) preserves the metadata/data split; migrated all five negative providers (the two new + DDL/DML/Batch from PR 4) to it. Kept compare() for the DML/Volume positive suites with a javadoc warning. Added foldInto unit tests. Validated live against the peco serverless warehouse (Thrift-vs-SEA, shadow): positive suites run in the same JVM AFTER the state-mutating negatives and still PASS (no shared-connection poisoning). Surfaced a real one-sided divergence: DDL-in-transaction returns false on one side, throws 42P07 on the other. ErrorComparatorTest: 20/20. Isaac Review: 0 findings. NO_CHANGELOG=true (test-only comparator tooling). Co-authored-by: Isaac Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
sreekanth-db
added a commit
that referenced
this pull request
Jul 3, 2026
## What Completes the isolation-heavy negative suites, on top of the `ConnectionFactory` mechanism from #1528. Builds on #1524–#1528. ### ConnectionFactory extension `NEGATIVE_CONNECTION` needs to build **deliberately-broken** connections, so the factory now also exposes `urlFor(side)`, `token()`, and `open(url, token)` (an uncached open with an arbitrary URL/token). Backed by `ConnectionManager.openUncached(url, token)` + `getToken()`. ### Suites - `NEGATIVE_CONNECTION` — opening *is* the test: starts from a side's healthy URL/token and corrupts one piece (bad token, unknown host, malformed httpPath, bad warehouse id, bad `ConnCatalog`/`ConnSchema`), then compares the connect failure. Closes any connection that unexpectedly opens. - `NEGATIVE_CANCEL_TIMEOUT` — `cancel()` mid slow query / after completion / on an already-cancelled statement, and `setQueryTimeout(1)` on a slow query. Own fresh connections; the slow query is a bounded range cross-join (~5s, no server-side sleep) to keep load and flakiness low. - `NEGATIVE_VOLUME` — GET/PUT/REMOVE failures + op-after-close. Runs under the `VOLUME_OPERATIONS` config (needs `VolumeOperationAllowedLocalPaths`), so it is excluded from `DEFAULT_PARAMS` like the positive VOLUME_OPERATIONS suite. ## Live validation Ran against the peco serverless warehouse (Thrift-vs-SEA, `ERROR_COMPARISON_MODE=shadow`). Positive suites run in the **same JVM after** the isolation-heavy negatives and still **PASS** — no shared-connection poisoning. - `NEGATIVE_CONNECTION` surfaced real error-shape divergences (bad token/host/warehouse). - `NEGATIVE_CANCEL_TIMEOUT` surfaced two one-sided findings: cancel-after-complete and double-cancel throw `STATEMENT_CLOSED` on one side and return normally on the other. - `NEGATIVE_VOLUME` is all-PASS — **confirmed meaningful (not vacuous)** via a throwaway probe: every case genuinely throws `DatabricksVolumeOperationException` / statement-closed **identically** on both endpoints, so PASS is real agreement. ## Testing `ErrorComparatorTest` — 20/20; live shadow runs (two, + volume probe); clean compile; Isaac Review (`/review --uncommitted`) — 0 findings. NO_CHANGELOG=true (test-only comparator tooling, no driver behavior change). This pull request and its description were written by Isaac. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
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
1 participant
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
Introduces the fresh-connection mechanism for negative suites whose cases mutate or destroy connection/session state, plus the first two such suites. Builds on #1524–#1527.
Mechanism
config/ConnectionFactory.openFresh(side)— a dedicated, uncached connectionConnectionManager.openUncached(url)—DriverManager.getConnection, not cachedSuiteProvider: backward-compatible default 5-argexecute(conn1, conn2, ConnectionFactory, tc, label)delegating to the 4-arg form — only isolation suites override it; the 12 existing providers are untouchedJDBCDriverComparisonTestbuilds a per-config factory bound to the resolved LEFT/RIGHT URLs and calls the 5-arg formSuites
Each opens its own fresh connections and closes them in a
finally, so they never poison the shared connections:NEGATIVE_CONNECTION_STATE—setCatalog/setSchema/setClientInfo/USEto a nonexistent targetNEGATIVE_TRANSACTION— commit/rollback with autocommit on; DDL inside a manual transaction (throwaway table dropped in afinally)Included fix: one-sided diff mis-filing
ErrorDiffs.comparereturns a flat list, so callers that dumped it intodataDifferencesmis-filed one-sided diffs (which belong inmetadataDifferences) and dropped them from the CSV summary — surfaced live as an empty summary on the transaction case. NewErrorDiffs.foldInto(result, ...)preserves the metadata/data split; migrated all five negative providers (the two new + DDL/DML/Batch from #1527) to it. Keptcompare()for the DML/Volume positive suites with a javadoc warning. AddedfoldIntounit tests.Live validation
Ran against the peco serverless warehouse (Thrift-vs-SEA,
ERROR_COMPARISON_MODE=shadow). Positive suites run in the same JVM after the state-mutating negatives and still PASS — no shared-connection poisoning. Surfaced a real one-sided divergence:DDL inside a manual transactionreturnsfalseon one side and throws42P07on the other.Testing
ErrorComparatorTest— 20/20 (addedfoldIntocoverage); two live shadow runs; clean compile; Isaac Review (/review --uncommitted) — 0 findings.NO_CHANGELOG=true (test-only comparator tooling, no driver behavior change).
This pull request and its description were written by Isaac.