From 04ef8a059b12e156a82ac92f66f839742257f221 Mon Sep 17 00:00:00 2001 From: Alex Kasko Date: Mon, 5 May 2025 18:23:03 +0100 Subject: [PATCH] Minor fix to executeBatch Currently the error message reported by `executeBatch` method is incorrect because this method tries to clear the batch on exit and such clearance is not possible when the statement is already closed after the error. This change adds a check that the statement is open before doing the cleanup. Testing: existing test is enhanced to check this error message; batch tests are moved to separate file. --- .../org/duckdb/DuckDBPreparedStatement.java | 4 +- src/test/java/org/duckdb/TestBatch.java | 127 ++++++++++++++++++ src/test/java/org/duckdb/TestDuckDBJDBC.java | 114 +--------------- src/test/java/org/duckdb/TestNumeric.java | 1 - 4 files changed, 131 insertions(+), 115 deletions(-) create mode 100644 src/test/java/org/duckdb/TestBatch.java diff --git a/src/main/java/org/duckdb/DuckDBPreparedStatement.java b/src/main/java/org/duckdb/DuckDBPreparedStatement.java index 520ec7557..12a2095cf 100644 --- a/src/main/java/org/duckdb/DuckDBPreparedStatement.java +++ b/src/main/java/org/duckdb/DuckDBPreparedStatement.java @@ -583,7 +583,9 @@ public long[] executeLargeBatch() throws SQLException { return executeBatchedStatements(); } } finally { - clearBatch(); + if (!isClosed()) { + clearBatch(); + } } } diff --git a/src/test/java/org/duckdb/TestBatch.java b/src/test/java/org/duckdb/TestBatch.java new file mode 100644 index 000000000..9f7bb2611 --- /dev/null +++ b/src/test/java/org/duckdb/TestBatch.java @@ -0,0 +1,127 @@ +package org.duckdb; + +import static org.duckdb.TestDuckDBJDBC.JDBC_URL; +import static org.duckdb.test.Assertions.*; + +import java.sql.*; + +public class TestBatch { + + public static void test_batch_prepared_statement() throws Exception { + try (Connection conn = DriverManager.getConnection(JDBC_URL)) { + try (Statement s = conn.createStatement()) { + s.execute("CREATE TABLE test (x INT, y INT, z INT)"); + } + try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (x, y, z) VALUES (?, ?, ?);")) { + ps.setObject(1, 1); + ps.setObject(2, 2); + ps.setObject(3, 3); + ps.addBatch(); + + ps.setObject(1, 4); + ps.setObject(2, 5); + ps.setObject(3, 6); + ps.addBatch(); + + ps.executeBatch(); + } + try (Statement s = conn.createStatement(); ResultSet rs = s.executeQuery("SELECT * FROM test ORDER BY x")) { + rs.next(); + assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); + assertEquals(rs.getObject(1, Integer.class), 1); + + assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); + assertEquals(rs.getObject(2, Integer.class), 2); + + assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); + assertEquals(rs.getObject(3, Integer.class), 3); + + rs.next(); + assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); + assertEquals(rs.getObject(1, Integer.class), 4); + + assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); + assertEquals(rs.getObject(2, Integer.class), 5); + + assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); + assertEquals(rs.getObject(3, Integer.class), 6); + } + } + } + + public static void test_batch_statement() throws Exception { + try (Connection conn = DriverManager.getConnection(JDBC_URL)) { + try (Statement s = conn.createStatement()) { + s.execute("CREATE TABLE test (x INT, y INT, z INT)"); + + s.addBatch("INSERT INTO test (x, y, z) VALUES (1, 2, 3);"); + s.addBatch("INSERT INTO test (x, y, z) VALUES (4, 5, 6);"); + + s.executeBatch(); + } + try (Statement s2 = conn.createStatement(); + ResultSet rs = s2.executeQuery("SELECT * FROM test ORDER BY x")) { + rs.next(); + assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); + assertEquals(rs.getObject(1, Integer.class), 1); + + assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); + assertEquals(rs.getObject(2, Integer.class), 2); + + assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); + assertEquals(rs.getObject(3, Integer.class), 3); + + rs.next(); + assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); + assertEquals(rs.getObject(1, Integer.class), 4); + + assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); + assertEquals(rs.getObject(2, Integer.class), 5); + + assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); + assertEquals(rs.getObject(3, Integer.class), 6); + } + } + } + + public static void test_execute_while_batch() throws Exception { + try (Connection conn = DriverManager.getConnection(JDBC_URL)) { + try (Statement s = conn.createStatement()) { + s.execute("CREATE TABLE test (id INT)"); + } + try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (id) VALUES (?)")) { + ps.setObject(1, 1); + ps.addBatch(); + + String msg = + assertThrows(() -> { ps.execute("INSERT INTO test (id) VALUES (1);"); }, SQLException.class); + assertTrue(msg.contains("Batched queries must be executed with executeBatch.")); + + String msg2 = + assertThrows(() -> { ps.executeUpdate("INSERT INTO test (id) VALUES (1);"); }, SQLException.class); + assertTrue(msg2.contains("Batched queries must be executed with executeBatch.")); + + String msg3 = assertThrows(() -> { ps.executeQuery("SELECT * FROM test"); }, SQLException.class); + assertTrue(msg3.contains("Batched queries must be executed with executeBatch.")); + } + } + } + + public static void test_prepared_statement_batch_exception() throws Exception { + try (Connection conn = DriverManager.getConnection(JDBC_URL)) { + try (Statement s = conn.createStatement()) { + s.execute("CREATE TABLE test (id INT)"); + } + try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (id) VALUES (?)")) { + String msg = assertThrows(() -> { ps.addBatch("DUMMY SQL"); }, SQLException.class); + assertTrue(msg.contains("Cannot add batched SQL statement to PreparedStatement")); + } + try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (id) VALUES (?)")) { + ps.setString(1, "foo"); + ps.addBatch(); + String msg = assertThrows(ps::executeBatch, SQLException.class); + assertTrue(msg.contains("Conversion Error: Could not convert string 'foo' to INT32")); + } + } + } +} diff --git a/src/test/java/org/duckdb/TestDuckDBJDBC.java b/src/test/java/org/duckdb/TestDuckDBJDBC.java index 6f402830a..08db903dd 100644 --- a/src/test/java/org/duckdb/TestDuckDBJDBC.java +++ b/src/test/java/org/duckdb/TestDuckDBJDBC.java @@ -3085,118 +3085,6 @@ public static void test_user_agent_custom() throws Exception { } } - public static void test_batch_prepared_statement() throws Exception { - try (Connection conn = DriverManager.getConnection(JDBC_URL)) { - try (Statement s = conn.createStatement()) { - s.execute("CREATE TABLE test (x INT, y INT, z INT)"); - } - try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (x, y, z) VALUES (?, ?, ?);")) { - ps.setObject(1, 1); - ps.setObject(2, 2); - ps.setObject(3, 3); - ps.addBatch(); - - ps.setObject(1, 4); - ps.setObject(2, 5); - ps.setObject(3, 6); - ps.addBatch(); - - ps.executeBatch(); - } - try (Statement s = conn.createStatement(); ResultSet rs = s.executeQuery("SELECT * FROM test ORDER BY x")) { - rs.next(); - assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); - assertEquals(rs.getObject(1, Integer.class), 1); - - assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); - assertEquals(rs.getObject(2, Integer.class), 2); - - assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); - assertEquals(rs.getObject(3, Integer.class), 3); - - rs.next(); - assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); - assertEquals(rs.getObject(1, Integer.class), 4); - - assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); - assertEquals(rs.getObject(2, Integer.class), 5); - - assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); - assertEquals(rs.getObject(3, Integer.class), 6); - } - } - } - - public static void test_batch_statement() throws Exception { - try (Connection conn = DriverManager.getConnection(JDBC_URL)) { - try (Statement s = conn.createStatement()) { - s.execute("CREATE TABLE test (x INT, y INT, z INT)"); - - s.addBatch("INSERT INTO test (x, y, z) VALUES (1, 2, 3);"); - s.addBatch("INSERT INTO test (x, y, z) VALUES (4, 5, 6);"); - - s.executeBatch(); - } - try (Statement s2 = conn.createStatement(); - ResultSet rs = s2.executeQuery("SELECT * FROM test ORDER BY x")) { - rs.next(); - assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); - assertEquals(rs.getObject(1, Integer.class), 1); - - assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); - assertEquals(rs.getObject(2, Integer.class), 2); - - assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); - assertEquals(rs.getObject(3, Integer.class), 3); - - rs.next(); - assertEquals(rs.getInt(1), rs.getObject(1, Integer.class)); - assertEquals(rs.getObject(1, Integer.class), 4); - - assertEquals(rs.getInt(2), rs.getObject(2, Integer.class)); - assertEquals(rs.getObject(2, Integer.class), 5); - - assertEquals(rs.getInt(3), rs.getObject(3, Integer.class)); - assertEquals(rs.getObject(3, Integer.class), 6); - } - } - } - - public static void test_execute_while_batch() throws Exception { - try (Connection conn = DriverManager.getConnection(JDBC_URL)) { - try (Statement s = conn.createStatement()) { - s.execute("CREATE TABLE test (id INT)"); - } - try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (id) VALUES (?)")) { - ps.setObject(1, 1); - ps.addBatch(); - - String msg = - assertThrows(() -> { ps.execute("INSERT INTO test (id) VALUES (1);"); }, SQLException.class); - assertTrue(msg.contains("Batched queries must be executed with executeBatch.")); - - String msg2 = - assertThrows(() -> { ps.executeUpdate("INSERT INTO test (id) VALUES (1);"); }, SQLException.class); - assertTrue(msg2.contains("Batched queries must be executed with executeBatch.")); - - String msg3 = assertThrows(() -> { ps.executeQuery("SELECT * FROM test"); }, SQLException.class); - assertTrue(msg3.contains("Batched queries must be executed with executeBatch.")); - } - } - } - - public static void test_prepared_statement_batch_exception() throws Exception { - try (Connection conn = DriverManager.getConnection(JDBC_URL)) { - try (Statement s = conn.createStatement()) { - s.execute("CREATE TABLE test (id INT)"); - } - try (PreparedStatement ps = conn.prepareStatement("INSERT INTO test (id) VALUES (?)")) { - String msg = assertThrows(() -> { ps.addBatch("DUMMY SQL"); }, SQLException.class); - assertTrue(msg.contains("Cannot add batched SQL statement to PreparedStatement")); - } - } - } - public static void test_get_binary_stream() throws Exception { try (Connection connection = DriverManager.getConnection("jdbc:duckdb:"); PreparedStatement s = connection.prepareStatement("select ?")) { @@ -3557,7 +3445,7 @@ public static void main(String[] args) throws Exception { statusCode = runTests(new String[0], clazz); } else { // extension installation fails on CI, Spatial test is temporary disabled - statusCode = runTests(args, TestDuckDBJDBC.class, TestClosure.class, + statusCode = runTests(args, TestDuckDBJDBC.class, TestBatch.class, TestClosure.class, TestExtensionTypes.class /*, TestSpatial.class */, TestParameterMetadata.class, TestPrepare.class, TestResults.class, TestTimestamp.class); } diff --git a/src/test/java/org/duckdb/TestNumeric.java b/src/test/java/org/duckdb/TestNumeric.java index 7337e3594..add1d13e2 100644 --- a/src/test/java/org/duckdb/TestNumeric.java +++ b/src/test/java/org/duckdb/TestNumeric.java @@ -1,7 +1,6 @@ package org.duckdb; import static org.duckdb.TestDuckDBJDBC.JDBC_URL; -import static org.duckdb.TestDuckDBJDBC.test_batch_prepared_statement; import static org.duckdb.test.Assertions.*; import java.math.BigDecimal;