[PECOBLR-2086] MST transaction tests — parameterized for SEA/Thrift by vikrantpuppala · Pull Request #1366 · databricks/databricks-jdbc

vikrantpuppala · 2026-04-02T06:54:35Z

Summary

Adds comprehensive MST (Multi-Statement Transaction) E2E test suite for the JDBC driver
Tests are parameterized across SEA and Thrift backends — every test runs on both
Two transaction modes: JDBC API (setAutoCommit/commit/rollback) and Explicit SQL (BEGIN TRANSACTION/COMMIT/ROLLBACK) share a common abstract base class
Covers known server-side bugs: LC-13424, LC-13425, LC-13427, LC-13428
See docs/mst-test-design.md for full design, operation categorization, class hierarchy, and E2E results

Test architecture

AbstractMstTestBase — 22 shared correctness tests (mode-agnostic, no @Test — invoked via parameterized wrappers)
JdbcApiTransactionTests — inherits shared + 10 API-specific tests (autocommit, isolation, PreparedStatement)
ExplicitSqlTransactionTests — inherits shared + 7 SQL-specific tests (BEGIN TRANSACTION, SET AUTOCOMMIT)
MstMetadataTests — metadata RPCs with backend-aware assertions (SEA: assertThrows, Thrift: non-transactional freshness)
MstBlockedSqlTests — MSTCheckRule-blocked SQL introspection + setCatalog/setSchema (assertThrows on both backends)
MstExecuteVariantTests — executeUpdate/executeBatch (SEA: silently poisons txn or throws, Thrift: works correctly)

E2E results (174 test executions)

169 pass, 4 skip, 0 errors
4 skips are due to 2 known driver bugs:
- SEA closes PreparedStatement after execute() — B.8 PreparedStatement reuse, B.9 getMetaData after execute
- Thrift getFunctions driver logging bug (IllegalFormatConversionException) — D.7

Key E2E findings

Thrift metadata RPCs (getColumns, getTables, etc.) are non-transactional — they bypass MST isolation and see concurrent DDL changes
SEA executeUpdate/executeLargeUpdate silently poison the transaction (commit fails later); executeBatch throws directly
getFunctions via SEA is not blocked by MSTCheckRule (SHOW FUNCTIONS IN CATALOG with StatementType.METADATA bypasses the rule)
getPrimaryKeys fails on both backends — server routes it through GET_FUNCTIONS which is blocked in MST
SEA closes Statement/PreparedStatement after each execute() — tests use executeSql() helper with fresh Statement per call

Test plan

Run full suite on SEA backend
Run full suite on Thrift backend
Verify blocked SQL tests pass on both backends (24/24)
Verify execute variant behavior (SEA: poisons txn, Thrift: correct counts)
Verify metadata RPC behavior (SEA: throws, Thrift: non-transactional)
Verify freshness tests (Thrift sees concurrent DDL changes)

NO_CHANGELOG=true

This pull request was AI-assisted by Isaac.

Adds 37 new E2E integration tests to TransactionTests.java covering gaps identified in MST + xDBC metadata audit (LC-13424, LC-13425, LC-13427, LC-13428): - executeUpdate/executeLargeUpdate/executeBatch in transactions - DatabaseMetaData operations (getColumns, getTables, etc.) in active txn - PreparedStatement metadata before/after execute in transaction - MSTCheckRule-blocked SQL (SHOW/DESCRIBE/information_schema) in MST - Allowed operations (setCatalog, setSchema, DESCRIBE TABLE) in MST - Connection close with pending transaction (implicit rollback) - DDL behavior in transactions (CREATE/DROP/ALTER TABLE) - PreparedStatement parameterized DML in transactions - Concurrent DDL + parameterized DML (stale schema) - Edge cases: empty txn, read-only txn, holdability, timeout, retry Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

- Add ignoreTransactions=0 to JDBC URL to enable MST - Fix testCloseConnectionWithPendingTransaction: accept both auto-commit and rollback behaviors on close - Fix testGetFunctionsInsideActiveTransaction: catch known driver logging bug (IllegalFormatConversionException in getFunctions) Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

- setCatalog/setSchema/DESCRIBE TABLE are all blocked in MST via Thrift (SetCatalogCommand, SetNamespaceCommand, DescribeRelation) — tests now expect the exception - getPrimaryKeys/getCrossReference may poison transaction in MST — wrapped in try-catch - testParameterizedDMLAfterConcurrentAlterTable: catch driver logger bug - testCommitWithoutActiveTransaction: remove message content assertion - testTransactionContinuesAfterAllowedMetadataOp: use DML-only operations Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

- Add transactionTests.yml workflow to run TransactionTests and ExplicitTransactionStatementTests on push to main (same pattern as concurrencyExecutionTests.yml) - Update both e2e test classes to read credentials from env vars (DATABRICKS_HOST, DATABRICKS_TOKEN, DATABRICKS_HTTP_PATH, etc.) with fallback to hardcoded defaults for local development - Fake service replay mode not yet possible for transaction tests due to LC-13424 (SEA INLINE disposition bug with MST commands) Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

…edentials" This reverts commit dc758c7.

Documents the test architecture for MST transaction tests: - Operation categorization (broken vs stale vs works) per backend - Parameterized test class hierarchy (SEA + Thrift) - Shared correctness tests via abstract base class - Specialized test classes for metadata, blocked SQL, execute variants - Migration plan from current unstructured tests Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Adds visual diagrams for: - Operation categorization (broken/stale/works) - Driver routing (SEA vs Thrift code paths) - Class hierarchy - Test execution flow across backends and transaction modes - Migration plan from current to new structure - Test count breakdown Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

vikrantpuppala · 2026-04-06T06:08:35Z

docs/mst-test-design.md

+| # | Test | Description | Expected |
+|---|---|---|---|
+| A.1 | `testDefaultAutoCommitIsTrue` | New connection defaults to autoCommit=true | Pass |
+| A.2 | `testCommitSingleInsert` | Disable autocommit → INSERT → commit → verify from separate conn | Pass |


why does it say disable autocommit? it should depend on if it's running via api or explicit mode??

vikrantpuppala · 2026-04-06T06:09:42Z

docs/mst-test-design.md

+| A.4 | `testRollbackSingleInsert` | Disable autocommit → INSERT → rollback → verify not persisted | Pass |
+| A.5 | `testMultipleSequentialTransactions` | 3 sequential txns (commit, commit, rollback) → verify only first two persist | Pass |
+| A.6 | `testAutoStartAfterCommit` | Commit txn1 → insert+rollback txn2 → only txn1 data persists | Pass |
+| A.7 | `testAutoStartAfterRollback` | Rollback txn1 → insert+commit txn2 → only txn2 data persists | Pass |


this should sit in ApiTests?

vikrantpuppala · 2026-04-06T06:12:49Z

docs/mst-test-design.md

+| A.15 | `testWriteConflictSingleTable` | Two concurrent txns on same table → first commits → second gets ConcurrentAppendException | Pass |
+| A.16 | `testWriteSkewProvesSnapshotIsolation` | Two concurrent txns on different tables → both commit → proves Snapshot Isolation | Pass |
+| A.17 | `testCommitWithoutActiveTxnThrows` | autocommit=true → commit() → expect exception | Pass |
+| A.18 | `testRollbackWithoutActiveTxnBehavior` | autocommit=true → rollback() → document behavior (JDBC throws, explicit SQL is no-op) | Pass |


are we going to test 17 and 18 with both api and explicit? and do we mean SET AUTOCOMMIT or BEGIN TRANSACTION when running explicit sql tests? also are there scenarios when we'd like to test both?

vikrantpuppala · 2026-04-06T06:14:34Z

docs/mst-test-design.md

+| A.16 | `testWriteSkewProvesSnapshotIsolation` | Two concurrent txns on different tables → both commit → proves Snapshot Isolation | Pass |
+| A.17 | `testCommitWithoutActiveTxnThrows` | autocommit=true → commit() → expect exception | Pass |
+| A.18 | `testRollbackWithoutActiveTxnBehavior` | autocommit=true → rollback() → document behavior (JDBC throws, explicit SQL is no-op) | Pass |
+| A.19 | `testSetAutoCommitDuringActiveTxnThrows` | In active txn → setAutoCommit(true) → expect exception | Pass |


again, will we try SET AUTOCOMMIT in case of explicit txn? and if not, shouldn't this be in api tests? imo explicit sql tests shouldn't even know about the SET command and they should just interact with transactions using BEGIN command

vikrantpuppala · 2026-04-06T06:14:57Z

docs/mst-test-design.md

+| A.18 | `testRollbackWithoutActiveTxnBehavior` | autocommit=true → rollback() → document behavior (JDBC throws, explicit SQL is no-op) | Pass |
+| A.19 | `testSetAutoCommitDuringActiveTxnThrows` | In active txn → setAutoCommit(true) → expect exception | Pass |
+| A.20 | `testUnsupportedIsolationLevel` | Set isolation to READ_UNCOMMITTED/READ_COMMITTED/SERIALIZABLE → expect exception | Pass |
+| A.21 | `testSupportedIsolationLevel` | Set isolation to REPEATABLE_READ → verify | Pass |


aren't 20 and 21 api tests?

vikrantpuppala · 2026-04-06T06:16:10Z

docs/mst-test-design.md

+| A.29 | `testPreparedStatementUpdate` | Insert → parameterized UPDATE in txn → commit → verify | Pass |
+| A.30 | `testPreparedStatementReuseAcrossTransactions` | Same PreparedStatement used in txn1 (commit) and txn2 (commit) → verify both rows | Pass |
+| A.31 | `testPreparedStatementGetMetaDataAfterExecute` | Execute PreparedStatement SELECT → getMetaData() → verify column count | Pass |
+| A.32 | `testGetParameterMetaData` | Create parameterized PreparedStatement → getParameterMetaData() → verify non-null | Pass |


i feel like there are duplications in the tests like maybe we're testing the same thing again and again or in different ways, is there scope to reduce the number of tests here?

vikrantpuppala · 2026-04-06T06:18:31Z

docs/mst-test-design.md

+| A.31 | `testPreparedStatementGetMetaDataAfterExecute` | Execute PreparedStatement SELECT → getMetaData() → verify column count | Pass |
+| A.32 | `testGetParameterMetaData` | Create parameterized PreparedStatement → getParameterMetaData() → verify non-null | Pass |
+
+### B. ExplicitSqlTransactionTests — special tests


we should similarly just have api tests as highlighted in previous comments

vikrantpuppala · 2026-04-06T06:21:17Z

docs/mst-test-design.md

+6. **Add** staleness tests (C.9, C.10) — new, didn't exist before
+7. **Parameterize** everything on SEA/Thrift
+
+### Tests being removed (and why)


why do we need to delete these? can we not add assertions instead?

- Move API-specific tests (setAutoCommit, isolation level) out of shared base into JdbcApiTransactionTests - Clarify ExplicitSqlTransactionTests uses BEGIN TRANSACTION only; SET AUTOCOMMIT tests cover implicit SQL mode as special cases - Remove duplicated tests (SET AUTOCOMMIT FALSE commit/rollback already covered by shared base via inherited tests) - Move setCatalog/setSchema to MstBlockedSqlTests (API calls, not shared correctness) - Make all shared test descriptions mode-agnostic (startTransaction instead of disable autocommit) - Keep DDL/empty-commit/retry tests as needing E2E validation instead of deleting them - Update class diagram, test counts, and execution flow diagrams Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

vikrantpuppala · 2026-04-06T07:05:04Z

docs/mst-test-design.md

+| A.5 | `testAutoStartAfterCommit` | Commit txn1 → insert+rollback txn2 → only txn1 data persists | Pass |
+| A.6 | `testAutoStartAfterRollback` | Rollback txn1 → insert+commit txn2 → only txn2 data persists | Pass |


aren't these only applicable to setAutocommit and hence should be in api tests?

Auto-start after commit/rollback is specific to implicit transaction mode (setAutoCommit=false). With BEGIN TRANSACTION there is no auto-start — you need an explicit BEGIN for each transaction. Moved testAutoStartAfterCommit and testAutoStartAfterRollback from shared base (22 tests) to JdbcApiTransactionTests (now 10 API-specific tests). Updated class diagram, execution flow, and counts accordingly. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Replace TransactionTests.java and ExplicitTransactionStatementTests.java with a structured, parameterized test hierarchy in the mst/ package: - AbstractMstTestBase: 22 shared correctness tests (mode-agnostic), parameterized by backend (SEA/Thrift) - JdbcApiTransactionTests: inherits shared + 10 API-specific tests (autocommit, isolation level, auto-start, PreparedStatement metadata) - ExplicitSqlTransactionTests: inherits shared + 7 SQL-specific tests (BEGIN TRANSACTION, SET AUTOCOMMIT command) - MstMetadataTests: 10 tests for metadata RPCs in MST with backend-aware assertions (SEA: xfail, Thrift: staleness) - MstBlockedSqlTests: 12 tests for MSTCheckRule-blocked SQL and API calls, verifying both exception and transaction abort - MstExecuteVariantTests: 4 tests for executeUpdate/executeBatch row counts (Thrift: correct, SEA: TBD after E2E) Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

@test

- Remove @test from AbstractMstTestBase methods (JUnit was running them directly on gap test subclasses without initialization) - Use separate Statement objects for DROP and CREATE in createTestTable (Databricks JDBC driver closes statement after execution) - Default catalog/schema to main/default when env vars not set Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

SEA backend auto-closes statements after execution, so reusing a Statement for multiple execute() calls fails. Refactored all tests to use executeSql() helper which creates a fresh Statement per call. Also use unique table names per test class (based on class name) to allow parallel test execution without table collisions. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

MstMetadataTests: - Each test now starts its own fresh transaction (was sharing one) - getFunctions: both backends return ResultSet (not blocked by MSTCheckRule when issued with StatementType.METADATA). Thrift path catches known driver logging bug (IllegalFormatConversionException) - Staleness tests: flip assertions — Thrift RPCs ARE non-transactional and DO see concurrent DDL (proving they bypass transaction isolation) MstExecuteVariantTests: - SEA: wrap execute calls in assertThrows (LC-13424 confirmed — executeUpdate/executeBatch abort the transaction on SEA) - Thrift: keep existing correctness assertions JdbcApiTransactionTests: - PreparedStatementUpdate: use execute() instead of executeUpdate() to avoid LC-13424 on SEA Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

E2E shows executeUpdate and executeLargeUpdate work on SEA (return row count, don't abort transaction). Only executeBatch aborts on SEA. Use same assertion for both backends — verify commit and data persist. Row count may be stale on SEA (LC-13424). Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

E2E confirms executeUpdate and executeLargeUpdate on SEA succeed (return a row count) but silently poison the transaction — the subsequent commit() fails with TRANSACTION_ROLLBACK_REQUIRED. Assert this behavior: SEA expects commit to throw, Thrift expects normal commit and data persistence. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

SEA backend closes PreparedStatement after execute(), preventing reuse across transactions and getMetaData() after execution. Skip these tests on SEA with Assumptions.assumeTrue(isThrift()). Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Major updates based on actual E2E results: - Thrift metadata RPCs are non-transactional (see concurrent DDL), not stale — flipped terminology and assertions - executeUpdate/executeLargeUpdate on SEA silently poison txn (commit fails), don't throw directly. executeBatch throws. - getFunctions not blocked on SEA (StatementType.METADATA bypass) - SEA closes PreparedStatement after execute — documented as implementation detail affecting test design - Added E2E results table, known issues section - Updated all diagrams to reflect verified behavior - Removed TBD/open items that are now resolved Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

E2E confirms getPrimaryKeys Thrift RPC also fails inside MST — the server routes TGetPrimaryKeysReq through GET_FUNCTIONS which is blocked. Changed from Thrift-pass/SEA-throw to assertThrows on both backends. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

…unts - getPrimaryKeys fails on both backends (server routes through GET_FUNCTIONS), not session poisoning — updated categorization - Fixed E2E results: 169 pass, 5 skip, 0 errors - Updated known issues: removed session poisoning, added getPrimaryKeys server routing issue - Corrected skip count: 3 in MstMetadataTests (D.7 Thrift, D.9 SEA, D.10 SEA) + 2 in JdbcApiTransactionTests (B.8, B.9) Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

vikrantpuppala added 8 commits March 26, 2026 09:32

Rename test table to avoid collisions with Python driver tests

2929c4d

Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Revert "Add CI workflow for transaction tests and use env vars for cr…

18a9fb2

…edentials" This reverts commit dc758c7.

vikrantpuppala commented Apr 6, 2026

View reviewed changes

vikrantpuppala changed the title ~~[PECOBLR-2086] Add comprehensive MST transaction metadata and edge case tests~~ [PECOBLR-2086] MST transaction tests — parameterized for SEA/Thrift Apr 6, 2026

vikrantpuppala added 10 commits April 6, 2026 07:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PECOBLR-2086] MST transaction tests — parameterized for SEA/Thrift#1366

[PECOBLR-2086] MST transaction tests — parameterized for SEA/Thrift#1366
vikrantpuppala wants to merge 20 commits intodatabricks:mainfrom
vikrantpuppala:add-mst-transaction-tests

vikrantpuppala commented Apr 2, 2026 •

edited

Loading

Uh oh!

vikrantpuppala Apr 6, 2026 •

edited by msrathore-db

Loading

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

vikrantpuppala Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		\| A.5 \| `testAutoStartAfterCommit` \| Commit txn1 → insert+rollback txn2 → only txn1 data persists \| Pass \|
		\| A.6 \| `testAutoStartAfterRollback` \| Rollback txn1 → insert+commit txn2 → only txn2 data persists \| Pass \|

Conversation

vikrantpuppala commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test architecture

E2E results (174 test executions)

Key E2E findings

Test plan

Uh oh!

vikrantpuppala Apr 6, 2026 • edited by msrathore-db Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vikrantpuppala commented Apr 2, 2026 •

edited

Loading

vikrantpuppala Apr 6, 2026 •

edited by msrathore-db

Loading