feat(duckdb): failure diagnosis, file create/open, read-only mode and faster JDBC connects

jdbc-bridge/src/main/java/sqlkit/bridge/ConnectionManager.java

@@ -15,6 +15,17 @@ public class ConnectionManager {
     private final Map<String, HikariDataSource> pools = new HashMap<>();
     private final Map<String, DriverClassLoader> loaders = new HashMap<>();
 
+    /**
+     * Driver classes loaded once per JAR set.
+     *
+     * A fresh class loader per connection re-runs the driver's static
+     * initialisation, which for DuckDB maps a ~107 MB native library (measured
+     * ~1.4 s). Caching the loader keeps that cost to the first connection of the
+     * bridge process; entries are released together with the process.
+     */
+    private final Map<String, DriverClassLoader> sharedLoaders = new HashMap<>();
+    private final Map<String, Driver> sharedDrivers = new HashMap<>();
+
     /**
      * Create a new JDBC connection pool.
      *
@@ -49,15 +60,16 @@ public void connect(String connId, String url, String username,
         }
         final String jdbcUrl = url;
 
-        DriverClassLoader loader = new DriverClassLoader(driverJars);
-        Class<?> driverCls = Class.forName(driverClass, true, loader);
-        if (!java.sql.Driver.class.isAssignableFrom(driverCls)) {
-            throw new ClassifiedException("Class " + driverClass + " does not implement java.sql.Driver", null, ErrorClassifier.ErrorType.UNKNOWN);
-        }
-        final java.sql.Driver driver = (java.sql.Driver) driverCls.getDeclaredConstructor().newInstance();
+        final java.sql.Driver driver = sharedDriver(driverClass, driverJars);
+        DriverClassLoader loader = loaderFor(driverJars);
         loaders.put(connId, loader);
 
         HikariConfig config = new HikariConfig();
+        // Keep HikariCP's read-only flag aligned with the connection the driver
+        // already opened: HikariCP calls `connection.setReadOnly(config.isReadOnly())`
+        // whenever the two disagree, and DuckDB rejects changing the mode at
+        // connection level ("Can't change read-only status on connection level").
+        config.setReadOnly(isReadOnlyUrl(jdbcUrl));
         config.setDataSource(new javax.sql.DataSource() {
             public java.sql.Connection getConnection() throws java.sql.SQLException {
                 java.util.Properties info = new java.util.Properties();
@@ -106,6 +118,51 @@ public void setLoginTimeout(int seconds) {}
         pools.put(connId, ds);
     }
 
+    /** Reuse one {@link DriverClassLoader} per set of driver JARs. */
+    private DriverClassLoader loaderFor(List<String> driverJars) throws Exception {
+        String key = String.join("|", driverJars);
+        DriverClassLoader loader = sharedLoaders.get(key);
+        if (loader == null) {
+            loader = new DriverClassLoader(driverJars);
+            sharedLoaders.put(key, loader);
+        }
+        return loader;
+    }
+
+    /** Reuse the instantiated JDBC driver for a driver class + JAR set. */
+    private Driver sharedDriver(String driverClass, List<String> driverJars) throws Exception {
+        String key = driverClass + "|" + String.join("|", driverJars);
+        Driver driver = sharedDrivers.get(key);
+        if (driver != null) {
+            return driver;
+        }
+
+        DriverClassLoader loader = loaderFor(driverJars);
+        Class<?> driverCls = Class.forName(driverClass, true, loader);
+        if (!java.sql.Driver.class.isAssignableFrom(driverCls)) {
+            throw new ClassifiedException("Class " + driverClass + " does not implement java.sql.Driver", null, ErrorClassifier.ErrorType.UNKNOWN);
+        }
+        driver = (java.sql.Driver) driverCls.getDeclaredConstructor().newInstance();
+        sharedDrivers.put(key, driver);
+        return driver;
+    }
+
+    /**
+     * Whether a JDBC URL asks for a read-only database.
+     *
+     * DuckDB expresses this in the URL (`access_mode=READ_ONLY` from SQLKit, or
+     * the driver property `duckdb.read_only`), and the driver refuses to change
+     * the mode through {@link java.sql.Connection#setReadOnly(boolean)}.
+     */
+    static boolean isReadOnlyUrl(String jdbcUrl) {
+        if (jdbcUrl == null) {
+            return false;
+        }
+        String normalized = jdbcUrl.toLowerCase(Locale.ROOT);
+        return normalized.contains("access_mode=read_only")
+                || normalized.contains("duckdb.read_only=true");
+    }
+
     /**
      * Close and remove a connection pool.
      */
@@ -114,10 +171,9 @@ public void disconnect(String connId) {
         if (ds != null) {
             ds.close();
         }
-        DriverClassLoader loader = loaders.remove(connId);
-        if (loader != null) {
-            try { loader.close(); } catch (Exception ignored) { }
-        }
+        // The loader is shared with other connections (see sharedLoaders), so it is
+        // only closed when the whole bridge shuts down.
+        loaders.remove(connId);
     }
 
     /**
@@ -164,5 +220,10 @@ public void closeAll() {
             try { loader.close(); } catch (Exception ignored) { }
         }
         loaders.clear();
+        for (DriverClassLoader loader : sharedLoaders.values()) {
+            try { loader.close(); } catch (Exception ignored) { }
+        }
+        sharedLoaders.clear();
+        sharedDrivers.clear();
     }
 }

jdbc-bridge/src/test/java/sqlkit/bridge/ConnectionManagerTest.java

@@ -0,0 +1,24 @@
+package sqlkit.bridge;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class ConnectionManagerTest {
+
+    @Test
+    void detectsReadOnlyJdbcUrls() {
+        assertTrue(ConnectionManager.isReadOnlyUrl("/tmp/app.duckdb;access_mode=READ_ONLY"));
+        assertTrue(ConnectionManager.isReadOnlyUrl("jdbc:duckdb:/data/app.duckdb;ACCESS_MODE=read_only"));
+        assertTrue(ConnectionManager.isReadOnlyUrl("jdbc:duckdb:/data/app.duckdb;duckdb.read_only=true"));
+    }
+
+    @Test
+    void treatsEverythingElseAsReadWrite() {
+        assertFalse(ConnectionManager.isReadOnlyUrl("jdbc:duckdb:/data/app.duckdb"));
+        assertFalse(ConnectionManager.isReadOnlyUrl("jdbc:duckdb:memory:shared"));
+        assertFalse(ConnectionManager.isReadOnlyUrl("jdbc:duckdb:/data/app.duckdb;duckdb.read_only=false"));
+        assertFalse(ConnectionManager.isReadOnlyUrl(null));
+    }
+}

scripts/duckdb-jdbc-verify/Verify.java

@@ -0,0 +1,164 @@
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Properties;
+
+/**
+ * Verifies the DuckDB JDBC semantics that SQLKit's connection form and JDBC URL
+ * building depend on. Run it with the DuckDB JDBC driver on the classpath:
+ *
+ *   scripts/verify-duckdb-jdbc-urls.sh
+ *
+ * Expectations encoded here (DuckDB 1.5.x):
+ *   - a missing database file is created by the engine on connect
+ *   - `;access_mode=READ_ONLY` works as a URL option and blocks writes
+ *   - the same file opened twice with the SAME configuration shares one instance
+ *   - the same file opened twice with DIFFERENT configurations is rejected
+ *   - a NAMED in-memory instance (`memory:<label>`) is shared between connections,
+ *     while an unnamed one is private to each connection (this is why SQLKit
+ *     stores `memory:<label>` as the host of an in-memory connection)
+ *   - a database path containing `;` cannot be expressed in the JDBC URL at all
+ */
+public class Verify {
+    private static int failures = 0;
+
+    private static void check(String name, boolean ok, String detail) {
+        if (!ok) {
+            failures++;
+        }
+        System.out.println((ok ? "PASS" : "FAIL") + " | " + name + (detail.isEmpty() ? "" : " | " + detail));
+    }
+
+    public static void main(String[] args) throws Exception {
+        Path tmp = Files.createTempDirectory("duckdb-verify");
+        Path cwd = Paths.get("").toAbsolutePath();
+        Class.forName("org.duckdb.DuckDBDriver");
+
+        // 1. A missing file is created by the engine on connect.
+        Path db = tmp.resolve("auto.duckdb");
+        try (Connection c = DriverManager.getConnection("jdbc:duckdb:" + db);
+             Statement s = c.createStatement()) {
+            s.execute("CREATE TABLE t AS SELECT 42 AS x");
+        }
+        check("missing file auto-created on connect", Files.exists(db), "size=" + Files.size(db));
+
+        byte[] header = Files.readAllBytes(db);
+        check("DUCK magic at byte offset 8", new String(header, 8, 4, "US-ASCII").equals("DUCK"), "");
+
+        // 2. URL option: read-only connects and rejects writes.
+        try (Connection ro = DriverManager.getConnection("jdbc:duckdb:" + db + ";access_mode=READ_ONLY");
+             Statement s = ro.createStatement();
+             ResultSet rs = s.executeQuery("SELECT count(*) FROM t")) {
+            rs.next();
+            check("URL option access_mode=READ_ONLY connects", rs.getInt(1) == 1, "count=" + rs.getInt(1));
+            boolean rejected = false;
+            try (Statement w = ro.createStatement()) {
+                w.execute("CREATE TABLE nope AS SELECT 1");
+            } catch (SQLException e) {
+                rejected = true;
+            }
+            check("read-only connection rejects writes", rejected, "");
+        }
+
+        // 3. The equivalent driver property still works as a fallback path.
+        Properties props = new Properties();
+        props.setProperty("duckdb.read_only", "true");
+        try (Connection ignored = DriverManager.getConnection("jdbc:duckdb:" + db, props)) {
+            check("duckdb.read_only property connects", true, "");
+        } catch (SQLException e) {
+            check("duckdb.read_only property connects", false, e.getMessage());
+        }
+
+        // 4. Different configurations for one file in one JVM are rejected; this is
+        //    what the connection form blocks before the engine ever sees it.
+        try (Connection rw = DriverManager.getConnection("jdbc:duckdb:" + db)) {
+            try (Connection ro = DriverManager.getConnection("jdbc:duckdb:" + db + ";access_mode=READ_ONLY")) {
+                check("conflicting configuration rejected", false, "second connection unexpectedly succeeded");
+            } catch (SQLException e) {
+                check("conflicting configuration rejected", true, e.getMessage());
+            }
+        }
+
+        // 5. Same file, same configuration -> one shared in-process instance.
+        try (Connection a = DriverManager.getConnection("jdbc:duckdb:" + db);
+             Connection b = DriverManager.getConnection("jdbc:duckdb:" + db);
+             Statement s = a.createStatement()) {
+            s.execute("CREATE TABLE IF NOT EXISTS shared_rw AS SELECT 1 AS x");
+            try (Statement s2 = b.createStatement();
+                 ResultSet rs = s2.executeQuery("SELECT count(*) FROM shared_rw")) {
+                rs.next();
+                check("same file shares one in-process instance", rs.getInt(1) == 1, "");
+            }
+        }
+
+        // 6. Named in-memory instances are shared across pooled connections.
+        try (Connection a = DriverManager.getConnection("jdbc:duckdb:memory:sqlkit_verify");
+             Connection b = DriverManager.getConnection("jdbc:duckdb:memory:sqlkit_verify");
+             Statement s = a.createStatement()) {
+            s.execute("CREATE TABLE shared_mem AS SELECT 1 AS x");
+            boolean visible = false;
+            try (Statement s2 = b.createStatement();
+                 ResultSet rs = s2.executeQuery("SELECT count(*) FROM shared_mem")) {
+                rs.next();
+                visible = rs.getInt(1) == 1;
+            }
+            check("named memory instance shared across connections", visible, "");
+        }
+
+        // 7. Unnamed in-memory instances are private to a connection: the reason
+        //    SQLKit must never store a bare `:memory:` for a pooled DuckDB pool.
+        try (Connection a = DriverManager.getConnection("jdbc:duckdb:");
+             Connection b = DriverManager.getConnection("jdbc:duckdb:");
+             Statement s = a.createStatement()) {
+            s.execute("CREATE TABLE private_mem AS SELECT 1 AS x");
+            boolean visible = true;
+            try (Statement s2 = b.createStatement();
+                 ResultSet rs = s2.executeQuery("SELECT count(*) FROM private_mem")) {
+                rs.next();
+            } catch (SQLException e) {
+                visible = false;
+            }
+            check("unnamed memory instance is connection-private", !visible, "");
+        }
+
+        // 8. The legacy `:memory:` database value resolves to an in-memory database
+        //    and does not create a literal file.
+        try (Connection c = DriverManager.getConnection("jdbc:duckdb::memory:");
+             Statement s = c.createStatement();
+             ResultSet rs = s.executeQuery("SELECT current_database()")) {
+            rs.next();
+            check("legacy jdbc:duckdb::memory: connects", true, "current_database=" + rs.getString(1));
+        } catch (SQLException e) {
+            check("legacy jdbc:duckdb::memory: connects", false, e.getMessage());
+        }
+        check("no literal ':memory:' file created in cwd", !Files.exists(cwd.resolve(":memory:")), "");
+
+        // 9. A path containing ';' cannot be expressed: the driver splits the URL on
+        //    ';' and reports `Invalid URL entry`. SQLKit rejects such paths.
+        Path semicolon = tmp.resolve("semi;colon.duckdb");
+        boolean semicolonFailed = false;
+        try (Connection ignored = DriverManager.getConnection("jdbc:duckdb:" + semicolon)) {
+            semicolonFailed = false;
+        } catch (SQLException e) {
+            semicolonFailed = true;
+        }
+        check("path containing ';' is unusable in a JDBC URL", semicolonFailed, "");
+
+        Path equals = tmp.resolve("eq=uals.duckdb");
+        try (Connection ignored = DriverManager.getConnection("jdbc:duckdb:" + equals)) {
+            check("path containing '=' opens as a file", Files.exists(equals), "");
+        } catch (SQLException e) {
+            check("path containing '=' opens as a file", false, e.getMessage());
+        }
+
+        System.out.println(failures == 0 ? "ALL CHECKS PASSED" : failures + " CHECK(S) FAILED");
+        if (failures > 0) {
+            System.exit(1);
+        }
+    }
+}

scripts/verify-duckdb-jdbc-urls.sh

@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+#
+# Verifies the DuckDB JDBC URL / instance semantics that SQLKit's connection
+# form relies on (file auto-creation, read-only, named in-memory instances,
+# configuration conflicts). Requires a JDK 11+ on PATH.
+#
+# Usage: scripts/verify-duckdb-jdbc-urls.sh [driver-version]
+#
+set -euo pipefail
+
+DRIVER_VERSION="${1:-1.5.6.0}"
+GROUP_PATH="org/duckdb/duckdb_jdbc"
+JAR_NAME="duckdb_jdbc-${DRIVER_VERSION}.jar"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+WORK_DIR="${TMPDIR:-/tmp}/sqlkit-duckdb-verify"
+JAR_PATH="${WORK_DIR}/${JAR_NAME}"
+
+if ! command -v java >/dev/null 2>&1; then
+  echo "java is required to run this verification (JDK 11+)" >&2
+  exit 1
+fi
+
+mkdir -p "${WORK_DIR}"
+if [ ! -f "${JAR_PATH}" ]; then
+  echo "Downloading ${JAR_NAME} from Maven Central..."
+  curl -sSL --max-time 600 \
+    -o "${JAR_PATH}" \
+    "https://repo1.maven.org/maven2/${GROUP_PATH}/${DRIVER_VERSION}/${JAR_NAME}"
+fi
+
+echo "Running DuckDB JDBC checks with driver ${DRIVER_VERSION}"
+java -cp "${JAR_PATH}" "${SCRIPT_DIR}/duckdb-jdbc-verify/Verify.java"