From 3c8163226a88c03c609416eb7edee1151083745f Mon Sep 17 00:00:00 2001 From: Sreekanth Vadigi Date: Fri, 3 Jul 2026 11:14:50 +0000 Subject: [PATCH] Add own-namespace negative comparator suites: DDL/DML/BATCH (PR 4/7) Adds the error-provoking DDL, DML, and batch suites. Each compares the two endpoints' error behavior (class, SQLState, vendor code, message) via the gate-aware ErrorDiffs bridge, under the shadow default. - NEGATIVE_STATEMENT_DDL: DROP/CREATE/ALTER on missing or duplicate objects (+/- IF EXISTS), missing schema/catalog, malformed syntax - NEGATIVE_STATEMENT_DML: type-mismatch INSERT, NOT NULL violation, DML on a missing table, executeUpdate on a SELECT (wrong method), write overflow - NEGATIVE_STATEMENT_BATCH: executeBatch partial/full failure and a ResultSet-producing command; when both sides throw BatchUpdateException, also compares the per-element update counts Isolation: these suites mutate catalog objects, so each runs in its own namespace under comparator_ddl_tests (seeded fresh and dropped in a finally), keeping them safe on the shared connections. Validated live against the peco serverless warehouse (Thrift-vs-SEA, shadow) across two runs: 21/21 cases, identical results, and positive STATEMENT_DDL/STATEMENT_DML run in the same JVM still PASS afterward (no shared-connection poisoning). Also fixes an NPE in the shared ErrorDiffs.compare: the neither-threw case is now guarded centrally (returns empty, mirroring the deep path's NOT_APPLICABLE) before reaching legacyClassOnly, which dereferences the null error() of a RETURNED outcome. The new suites call ErrorDiffs.compare unconditionally, so an off-mode both-succeed case (e.g. DROP TABLE IF EXISTS) would otherwise NPE. Added a regression test. Registered the three suites in the TestSuite enum (auto-included under DEFAULT_PARAMS via allExcept). README suite table updated. NO_CHANGELOG=true (test-only comparator tooling). Co-authored-by: Isaac Signed-off-by: Sreekanth Vadigi --- .../jdbc/comparator/COMPARATOR_README.md | 3 + .../jdbc/comparator/config/TestSuite.java | 8 +- .../comparator/error/ErrorComparatorTest.java | 10 ++ .../jdbc/comparator/error/ErrorDiffs.java | 7 + .../suite/NegativeStatementBatchProvider.java | 145 ++++++++++++++++++ .../suite/NegativeStatementDdlProvider.java | 124 +++++++++++++++ .../suite/NegativeStatementDmlProvider.java | 120 +++++++++++++++ 7 files changed, 416 insertions(+), 1 deletion(-) create mode 100644 src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementBatchProvider.java create mode 100644 src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDdlProvider.java create mode 100644 src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDmlProvider.java diff --git a/src/test/java/com/databricks/jdbc/comparator/COMPARATOR_README.md b/src/test/java/com/databricks/jdbc/comparator/COMPARATOR_README.md index f9e5fa31c..880d6a35f 100644 --- a/src/test/java/com/databricks/jdbc/comparator/COMPARATOR_README.md +++ b/src/test/java/com/databricks/jdbc/comparator/COMPARATOR_README.md @@ -255,6 +255,9 @@ If both are present for a method, **runOnly takes precedence**: argument combina | `NEGATIVE_PARAM_BINDING` | Bad PreparedStatement bindings (index, count, type, precision) | | `NEGATIVE_PREPARED_METADATA` | clearParameters + unbound execute; getMetaData on invalid SQL | | `NEGATIVE_TYPE_CONVERSION` | Incompatible ResultSet.getX() conversions (overflow, wrong target) | +| `NEGATIVE_STATEMENT_DDL` | Error-provoking CREATE / ALTER / DROP (missing/duplicate objects, bad namespace, malformed) | +| `NEGATIVE_STATEMENT_DML` | Error-provoking INSERT / UPDATE / DELETE (type mismatch, NOT NULL, missing table, overflow) | +| `NEGATIVE_STATEMENT_BATCH` | executeBatch partial/full failure + per-element BatchUpdateException counts | Negative suites compare each endpoint's **error behavior** (exception class, SQLState, vendor code, message) via the `ERROR_COMPARISON_MODE` gate (default `shadow`). See diff --git a/src/test/java/com/databricks/jdbc/comparator/config/TestSuite.java b/src/test/java/com/databricks/jdbc/comparator/config/TestSuite.java index 8c8b8b3c4..b4d482a6e 100644 --- a/src/test/java/com/databricks/jdbc/comparator/config/TestSuite.java +++ b/src/test/java/com/databricks/jdbc/comparator/config/TestSuite.java @@ -27,7 +27,13 @@ public enum TestSuite { NEGATIVE_STATEMENT_OTHER(new NegativeStatementOtherProvider()), NEGATIVE_PARAM_BINDING(new NegativeParamBindingProvider()), NEGATIVE_PREPARED_METADATA(new NegativePreparedMetadataProvider()), - NEGATIVE_TYPE_CONVERSION(new NegativeTypeConversionProvider()); + NEGATIVE_TYPE_CONVERSION(new NegativeTypeConversionProvider()), + + // Negative suites that mutate catalog objects — isolated in their own namespace + // under comparator_ddl_tests (seeded fresh and dropped), so safe on the shared connections. + NEGATIVE_STATEMENT_DDL(new NegativeStatementDdlProvider()), + NEGATIVE_STATEMENT_DML(new NegativeStatementDmlProvider()), + NEGATIVE_STATEMENT_BATCH(new NegativeStatementBatchProvider()); private final SuiteProvider provider; diff --git a/src/test/java/com/databricks/jdbc/comparator/error/ErrorComparatorTest.java b/src/test/java/com/databricks/jdbc/comparator/error/ErrorComparatorTest.java index c34a15b7b..39a064e71 100644 --- a/src/test/java/com/databricks/jdbc/comparator/error/ErrorComparatorTest.java +++ b/src/test/java/com/databricks/jdbc/comparator/error/ErrorComparatorTest.java @@ -204,6 +204,16 @@ void errorDiffsOffModeStillFlagsDifferingClass() { }); } + @Test + void errorDiffsNeitherThrewIsEmptyInBothModes() { + // Both sides returned (e.g. DROP TABLE IF EXISTS succeeds on both) -> no diffs, no NPE. + // error() is null for a RETURNED outcome, so this guards the legacy-path null dereference. + CapturedOutcome l = CapturedOutcome.returned(0); + CapturedOutcome r = CapturedOutcome.returned(0); + withMode("off", () -> assertTrue(ErrorDiffs.compare(l, r, "v ").isEmpty())); + withMode("shadow", () -> assertTrue(ErrorDiffs.compare(l, r, "v ").isEmpty())); + } + private static void withMode(String mode, Runnable body) { String prev = System.getProperty("ERROR_COMPARISON_MODE"); try { diff --git a/src/test/java/com/databricks/jdbc/comparator/error/ErrorDiffs.java b/src/test/java/com/databricks/jdbc/comparator/error/ErrorDiffs.java index 9bc623263..25b93367e 100644 --- a/src/test/java/com/databricks/jdbc/comparator/error/ErrorDiffs.java +++ b/src/test/java/com/databricks/jdbc/comparator/error/ErrorDiffs.java @@ -24,6 +24,13 @@ private ErrorDiffs() {} */ public static List compare( CapturedOutcome left, CapturedOutcome right, String returnedLabel) { + // Neither side threw — nothing to compare here (mirrors the deep path's NOT_APPLICABLE). + // Guard centrally so every caller is safe, including callers that invoke compare() + // unconditionally (the negative DDL/DML/Batch suites); their both-succeed cases (e.g. + // DROP TABLE IF EXISTS) must not dereference the null error() in off mode. + if (!left.threw() && !right.threw()) { + return Collections.emptyList(); + } ErrorPolicy policy = ErrorPolicy.active(); if (!policy.isDeepComparisonEnabled()) { return legacyClassOnly(left, right); diff --git a/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementBatchProvider.java b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementBatchProvider.java new file mode 100644 index 000000000..75e6697e2 --- /dev/null +++ b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementBatchProvider.java @@ -0,0 +1,145 @@ +package com.databricks.jdbc.comparator.suite; + +import com.databricks.jdbc.comparator.ComparisonResult; +import com.databricks.jdbc.comparator.error.CapturedOutcome; +import com.databricks.jdbc.comparator.error.Captures; +import com.databricks.jdbc.comparator.error.ErrorDiffs; +import java.sql.BatchUpdateException; +import java.sql.Connection; +import java.sql.Statement; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Negative batch cases — {@code executeBatch()} with a failing command (partial failure, full + * failure, or a command that produces a ResultSet). Compares each endpoint's error behavior via + * {@link ErrorDiffs}, and — when both sides throw {@link BatchUpdateException} — also compares the + * per-element update counts, which are the JDBC-defined signal for partial batch failure. + * + *

Isolation: own namespace under {@code comparator_ddl_tests}, seeded fresh and dropped, so it + * is safe on the shared connections. + */ +public class NegativeStatementBatchProvider implements SuiteProvider { + + private static final String CATALOG = "comparator_ddl_tests"; + private static final String SCHEMA1 = CATALOG + ".neg_batch_thrift"; + private static final String SCHEMA2 = CATALOG + ".neg_batch_sea"; + + /** Adds the batch commands for one side, given that side's schema. */ + @FunctionalInterface + private interface BatchFor { + void add(Statement stmt, String schema) throws Exception; + } + + private static final class Case { + final String description; + final BatchFor batch; + + Case(String description, BatchFor batch) { + this.description = description; + this.batch = batch; + } + } + + private static final List CASES = + Arrays.asList( + new Case( + "executeBatch with one failing row (partial failure)", + (stmt, s) -> { + stmt.addBatch("INSERT INTO " + s + ".seed VALUES (10, 'ok')"); + stmt.addBatch("INSERT INTO " + s + ".seed VALUES ('bad', 'x')"); // type error + stmt.addBatch("INSERT INTO " + s + ".seed VALUES (11, 'ok2')"); + }), + new Case( + "executeBatch with all rows failing", + (stmt, s) -> { + stmt.addBatch("INSERT INTO " + s + ".__no_such__ VALUES (1)"); + stmt.addBatch("INSERT INTO " + s + ".__no_such__ VALUES (2)"); + }), + new Case( + "executeBatch where one command returns a ResultSet", + (stmt, s) -> { + stmt.addBatch("INSERT INTO " + s + ".seed VALUES (20, 'ok')"); + stmt.addBatch("SELECT * FROM " + s + ".seed"); // illegal in a batch + })); + + @Override + public List getTestCases() { + return CASES.stream() + .map(c -> new TestCase(c.description, c.description)) + .collect(Collectors.toList()); + } + + @Override + public ComparisonResult execute( + Connection conn1, Connection conn2, TestCase testCase, String label) throws Exception { + Case c = + CASES.stream() + .filter(x -> x.description.equals(testCase.getIdentifier())) + .findFirst() + .orElseThrow( + () -> new IllegalArgumentException("Unknown case: " + testCase.getIdentifier())); + + setup(conn1, SCHEMA1); + setup(conn2, SCHEMA2); + try (Statement s1 = conn1.createStatement(); + Statement s2 = conn2.createStatement()) { + // Building the batch is bookkeeping; only executeBatch() is the captured driver call. + c.batch.add(s1, SCHEMA1); + c.batch.add(s2, SCHEMA2); + CapturedOutcome left = Captures.capture(() -> s1.executeBatch()); + CapturedOutcome right = Captures.capture(() -> s2.executeBatch()); + + ComparisonResult result = new ComparisonResult(label, c.description, testCase.getArgs()); + for (String d : ErrorDiffs.compare(left, right, "batch counts ")) { + result.dataDifferences.add(d); + } + // When both threw BatchUpdateException, also compare the per-element update counts. + String counts = compareUpdateCounts(left, right); + if (counts != null) { + result.dataDifferences.add(counts); + } + return result; + } finally { + exec(conn1, "DROP SCHEMA IF EXISTS " + SCHEMA1 + " CASCADE"); + exec(conn2, "DROP SCHEMA IF EXISTS " + SCHEMA2 + " CASCADE"); + } + } + + /** + * Returns a diff string if both sides threw BatchUpdateException with differing counts, else + * null. + */ + private static String compareUpdateCounts(CapturedOutcome left, CapturedOutcome right) { + if (!left.threw() || !right.threw()) { + return null; + } + if (left.throwable() instanceof BatchUpdateException + && right.throwable() instanceof BatchUpdateException) { + int[] c1 = ((BatchUpdateException) left.throwable()).getUpdateCounts(); + int[] c2 = ((BatchUpdateException) right.throwable()).getUpdateCounts(); + if (!Arrays.equals(c1, c2)) { + return "Error batch updateCounts mismatch: " + + Arrays.toString(c1) + + " vs " + + Arrays.toString(c2); + } + } + return null; + } + + private void setup(Connection conn, String schema) { + exec(conn, "DROP SCHEMA IF EXISTS " + schema + " CASCADE"); + exec(conn, "CREATE SCHEMA " + schema); + exec(conn, "CREATE TABLE " + schema + ".seed (id INT, name STRING)"); + } + + private void exec(Connection conn, String sql) { + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate(sql); + } catch (Exception ignored) { + // best-effort setup/teardown + } + } +} diff --git a/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDdlProvider.java b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDdlProvider.java new file mode 100644 index 000000000..c672111c7 --- /dev/null +++ b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDdlProvider.java @@ -0,0 +1,124 @@ +package com.databricks.jdbc.comparator.suite; + +import com.databricks.jdbc.comparator.ComparisonResult; +import com.databricks.jdbc.comparator.error.CapturedOutcome; +import com.databricks.jdbc.comparator.error.Captures; +import com.databricks.jdbc.comparator.error.ErrorDiffs; +import java.sql.Connection; +import java.sql.Statement; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Negative DDL cases — CREATE / ALTER / DROP that should fail (missing or duplicate objects, + * missing namespaces, malformed syntax). Compares each endpoint's error behavior (class, SQLState, + * code, message) via {@link ErrorDiffs}, honoring the {@code ERROR_COMPARISON_MODE} gate. + * + *

Isolation: operates in its own namespace under {@code comparator_ddl_tests} (the same pattern + * as the positive DDL/DML suites), created fresh and dropped at the end, so it does not poison + * other suites even though it runs on the shared connections. Cases that must operate on an + * existing object (e.g. CREATE-already-exists, ALTER-add-duplicate-column) share a small seed + * table. + */ +public class NegativeStatementDdlProvider implements SuiteProvider { + + private static final String CATALOG = "comparator_ddl_tests"; + private static final String SCHEMA1 = CATALOG + ".neg_ddl_thrift"; + private static final String SCHEMA2 = CATALOG + ".neg_ddl_sea"; + + /** Produces the SQL for one side given that side's schema. */ + @FunctionalInterface + private interface SqlFor { + String sql(String schema); + } + + private static final class Case { + final String description; + final SqlFor sql; + + Case(String description, SqlFor sql) { + this.description = description; + this.sql = sql; + } + } + + private static final List CASES = + Arrays.asList( + new Case("DROP TABLE a non-existent table", s -> "DROP TABLE " + s + ".__no_such__"), + new Case( + "DROP TABLE IF EXISTS a non-existent table", + s -> "DROP TABLE IF EXISTS " + s + ".__no_such__"), + new Case("CREATE TABLE that already exists", s -> "CREATE TABLE " + s + ".seed (id INT)"), + new Case( + "CREATE TABLE IF NOT EXISTS that already exists", + s -> "CREATE TABLE IF NOT EXISTS " + s + ".seed (id INT)"), + new Case( + "ALTER TABLE a non-existent table", + s -> "ALTER TABLE " + s + ".__no_such__ ADD COLUMNS (x INT)"), + new Case( + "ALTER TABLE add a duplicate column", + s -> "ALTER TABLE " + s + ".seed ADD COLUMNS (id INT)"), + new Case( + "CREATE TABLE in a non-existent schema", + s -> "CREATE TABLE " + CATALOG + ".__no_such_schema__.t (id INT)"), + new Case( + "CREATE SCHEMA in a non-existent catalog", + s -> "CREATE SCHEMA __no_such_catalog__.s"), + new Case("Malformed DDL syntax", s -> "CREATE TABEL " + s + ".bad (id INT)")); + + @Override + public List getTestCases() { + return CASES.stream() + .map(c -> new TestCase(c.description, c.description)) + .collect(Collectors.toList()); + } + + @Override + public ComparisonResult execute( + Connection conn1, Connection conn2, TestCase testCase, String label) throws Exception { + Case c = + CASES.stream() + .filter(x -> x.description.equals(testCase.getIdentifier())) + .findFirst() + .orElseThrow( + () -> new IllegalArgumentException("Unknown case: " + testCase.getIdentifier())); + + // Setup a fresh seed table per side (bookkeeping — outside the captured call). + setup(conn1, SCHEMA1); + setup(conn2, SCHEMA2); + try { + CapturedOutcome left = Captures.capture(() -> execUpdate(conn1, c.sql.sql(SCHEMA1))); + CapturedOutcome right = Captures.capture(() -> execUpdate(conn2, c.sql.sql(SCHEMA2))); + ComparisonResult result = new ComparisonResult(label, c.description, testCase.getArgs()); + for (String d : ErrorDiffs.compare(left, right, "update count ")) { + result.dataDifferences.add(d); + } + return result; + } finally { + exec(conn1, "DROP SCHEMA IF EXISTS " + SCHEMA1 + " CASCADE"); + exec(conn2, "DROP SCHEMA IF EXISTS " + SCHEMA2 + " CASCADE"); + } + } + + private void setup(Connection conn, String schema) { + exec(conn, "DROP SCHEMA IF EXISTS " + schema + " CASCADE"); + exec(conn, "CREATE SCHEMA " + schema); + exec(conn, "CREATE TABLE " + schema + ".seed (id INT)"); + } + + private int execUpdate(Connection conn, String sql) throws Exception { + try (Statement stmt = conn.createStatement()) { + return stmt.executeUpdate(sql); + } + } + + /** Best-effort setup/teardown DDL; failures are ignored (this is bookkeeping, not the test). */ + private void exec(Connection conn, String sql) { + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate(sql); + } catch (Exception ignored) { + // best-effort + } + } +} diff --git a/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDmlProvider.java b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDmlProvider.java new file mode 100644 index 000000000..e73913178 --- /dev/null +++ b/src/test/java/com/databricks/jdbc/comparator/suite/NegativeStatementDmlProvider.java @@ -0,0 +1,120 @@ +package com.databricks.jdbc.comparator.suite; + +import com.databricks.jdbc.comparator.ComparisonResult; +import com.databricks.jdbc.comparator.error.CapturedOutcome; +import com.databricks.jdbc.comparator.error.Captures; +import com.databricks.jdbc.comparator.error.ErrorDiffs; +import java.sql.Connection; +import java.sql.Statement; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Negative DML cases — INSERT / UPDATE / DELETE that should fail (type mismatch, constraint + * violation, missing table, wrong method, non-updatable target, overflow). Compares each endpoint's + * error behavior via {@link ErrorDiffs}, honoring the {@code ERROR_COMPARISON_MODE} gate. + * + *

Isolation: operates in its own namespace under {@code comparator_ddl_tests}, seeded fresh per + * run and dropped at the end, so it is safe on the shared connections. + */ +public class NegativeStatementDmlProvider implements SuiteProvider { + + private static final String CATALOG = "comparator_ddl_tests"; + private static final String SCHEMA1 = CATALOG + ".neg_dml_thrift"; + private static final String SCHEMA2 = CATALOG + ".neg_dml_sea"; + + @FunctionalInterface + private interface SqlFor { + String sql(String schema); + } + + private static final class Case { + final String description; + final SqlFor sql; + + Case(String description, SqlFor sql) { + this.description = description; + this.sql = sql; + } + } + + private static final List CASES = + Arrays.asList( + new Case( + "INSERT a non-numeric string into an INT column", + s -> "INSERT INTO " + s + ".seed (id, name) VALUES ('not_an_int', 'x')"), + new Case( + "INSERT violating NOT NULL on id", + s -> "INSERT INTO " + s + ".seed (id, name) VALUES (NULL, 'x')"), + new Case( + "INSERT into a non-existent table", + s -> "INSERT INTO " + s + ".__no_such__ VALUES (1)"), + new Case( + "UPDATE a non-existent table", + s -> "UPDATE " + s + ".__no_such__ SET name = 'x' WHERE id = 1"), + new Case( + "DELETE from a non-existent table", + s -> "DELETE FROM " + s + ".__no_such__ WHERE id = 1"), + new Case("executeUpdate on a SELECT (wrong method)", s -> "SELECT * FROM " + s + ".seed"), + new Case( + "Write overflow: CAST('123456' AS DECIMAL(2,0))", + s -> + "INSERT INTO " + + s + + ".seed (id, name) VALUES (CAST('123456' AS DECIMAL(2,0)), 'x')")); + + @Override + public List getTestCases() { + return CASES.stream() + .map(c -> new TestCase(c.description, c.description)) + .collect(Collectors.toList()); + } + + @Override + public ComparisonResult execute( + Connection conn1, Connection conn2, TestCase testCase, String label) throws Exception { + Case c = + CASES.stream() + .filter(x -> x.description.equals(testCase.getIdentifier())) + .findFirst() + .orElseThrow( + () -> new IllegalArgumentException("Unknown case: " + testCase.getIdentifier())); + + setup(conn1, SCHEMA1); + setup(conn2, SCHEMA2); + try { + CapturedOutcome left = Captures.capture(() -> execUpdate(conn1, c.sql.sql(SCHEMA1))); + CapturedOutcome right = Captures.capture(() -> execUpdate(conn2, c.sql.sql(SCHEMA2))); + ComparisonResult result = new ComparisonResult(label, c.description, testCase.getArgs()); + for (String d : ErrorDiffs.compare(left, right, "update count ")) { + result.dataDifferences.add(d); + } + return result; + } finally { + exec(conn1, "DROP SCHEMA IF EXISTS " + SCHEMA1 + " CASCADE"); + exec(conn2, "DROP SCHEMA IF EXISTS " + SCHEMA2 + " CASCADE"); + } + } + + private void setup(Connection conn, String schema) { + exec(conn, "DROP SCHEMA IF EXISTS " + schema + " CASCADE"); + exec(conn, "CREATE SCHEMA " + schema); + exec(conn, "CREATE TABLE " + schema + ".seed (id INT NOT NULL, name STRING)"); + exec(conn, "INSERT INTO " + schema + ".seed VALUES (1, 'alice')"); + } + + private int execUpdate(Connection conn, String sql) throws Exception { + try (Statement stmt = conn.createStatement()) { + return stmt.executeUpdate(sql); + } + } + + private void exec(Connection conn, String sql) { + try (Statement stmt = conn.createStatement()) { + stmt.executeUpdate(sql); + } catch (Exception ignored) { + // best-effort setup/teardown + } + } +}