From 712a8a55f201954756de501a41a403d588b4d245 Mon Sep 17 00:00:00 2001 From: Gauthier Roebroeck Date: Wed, 16 Aug 2023 13:57:42 +0800 Subject: [PATCH] fix(jdbc): remove support for Statement#getGeneratedKeys SQLite does not provide a good way to retrieve generated keys in a reliable way BREAKING-CHANGE: getGeneratedKeys is not supported anymore Closes: #329 --- .../org/sqlite/core/CoreDatabaseMetaData.java | 12 +++++------- .../sqlite/jdbc3/JDBC3DatabaseMetaData.java | 13 +++++-------- .../java/org/sqlite/jdbc3/JDBC3Statement.java | 7 ++++--- src/test/java/org/sqlite/DBMetaDataTest.java | 1 - src/test/java/org/sqlite/StatementTest.java | 18 ++---------------- 5 files changed, 16 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/sqlite/core/CoreDatabaseMetaData.java b/src/main/java/org/sqlite/core/CoreDatabaseMetaData.java index 1c6a4fdf6..48805a493 100644 --- a/src/main/java/org/sqlite/core/CoreDatabaseMetaData.java +++ b/src/main/java/org/sqlite/core/CoreDatabaseMetaData.java @@ -43,9 +43,6 @@ public abstract class CoreDatabaseMetaData implements DatabaseMetaData { getVersionColumns = null, getColumnPrivileges = null; - /** Used to save generating a new statement every call. */ - protected PreparedStatement getGeneratedKeys = null; - /** * Constructor that applies the Connection object. * @@ -55,6 +52,11 @@ protected CoreDatabaseMetaData(SQLiteConnection conn) { this.conn = conn; } + /** + * @deprecated Not exactly sure what this function does, as it is not implementing any + * interface, and is not used anywhere in the code. Deprecated since 3.43.0.0. + */ + @Deprecated public abstract ResultSet getGeneratedKeys() throws SQLException; /** @throws SQLException */ @@ -122,9 +124,6 @@ public synchronized void close() throws SQLException { if (getColumnPrivileges != null) { getColumnPrivileges.close(); } - if (getGeneratedKeys != null) { - getGeneratedKeys.close(); - } getTables = null; getTableTypes = null; @@ -143,7 +142,6 @@ public synchronized void close() throws SQLException { getBestRowIdentifier = null; getVersionColumns = null; getColumnPrivileges = null; - getGeneratedKeys = null; } finally { conn = null; } diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java b/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java index 38c3dc513..a515ed04e 100644 --- a/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java +++ b/src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java @@ -556,7 +556,7 @@ public boolean supportsFullOuterJoins() throws SQLException { /** @see java.sql.DatabaseMetaData#supportsGetGeneratedKeys() */ public boolean supportsGetGeneratedKeys() { - return true; + return false; } /** @see java.sql.DatabaseMetaData#supportsGroupBy() */ @@ -1926,15 +1926,12 @@ public ResultSet getVersionColumns(String c, String s, String t) throws SQLExcep } /** - * @return Generated row id of the last INSERT command. - * @throws SQLException + * @deprecated Not exactly sure what this function does, as it is not implementing any + * interface, and is not used anywhere in the code. Deprecated since 3.43.0.0. */ + @Deprecated public ResultSet getGeneratedKeys() throws SQLException { - if (getGeneratedKeys == null) { - getGeneratedKeys = conn.prepareStatement("select last_insert_rowid();"); - } - - return getGeneratedKeys.executeQuery(); + throw new SQLFeatureNotSupportedException("not implemented by SQLite JDBC driver"); } /** Not implemented yet. */ diff --git a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java index 6acfd57ac..7be5ab586 100644 --- a/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java +++ b/src/main/java/org/sqlite/jdbc3/JDBC3Statement.java @@ -347,13 +347,14 @@ public void setFetchDirection(int direction) throws SQLException { } /** - * As SQLite's last_insert_rowid() function is DB-specific not statement specific, this function - * introduces a race condition if the same connection is used by two threads and both insert. + * SQLite's last_insert_rowid() function is DB-specific, not statement specific, and cannot + * provide multiple values when inserting multiple rows. Suggestion is to use a RETURNING clause instead. * * @see java.sql.Statement#getGeneratedKeys() */ public ResultSet getGeneratedKeys() throws SQLException { - return conn.getSQLiteDatabaseMetaData().getGeneratedKeys(); + throw unsupported(); } /** diff --git a/src/test/java/org/sqlite/DBMetaDataTest.java b/src/test/java/org/sqlite/DBMetaDataTest.java index 0c8ef7c83..ab6661c06 100644 --- a/src/test/java/org/sqlite/DBMetaDataTest.java +++ b/src/test/java/org/sqlite/DBMetaDataTest.java @@ -52,7 +52,6 @@ public void getTables() throws SQLException { ResultSet rs = meta.getTables(null, null, null, null); assertThat(rs).isNotNull(); - stat.getGeneratedKeys().close(); stat.close(); assertThat(rs.next()).isTrue(); diff --git a/src/test/java/org/sqlite/StatementTest.java b/src/test/java/org/sqlite/StatementTest.java index a749a50fe..dc7a7aa93 100644 --- a/src/test/java/org/sqlite/StatementTest.java +++ b/src/test/java/org/sqlite/StatementTest.java @@ -306,23 +306,9 @@ public void getGeneratedKeys() throws SQLException { ResultSet rs; stat.executeUpdate("create table t1 (c1 integer primary key, v);"); stat.executeUpdate("insert into t1 (v) values ('red');"); - rs = stat.getGeneratedKeys(); - assertThat(rs.next()).isTrue(); - assertThat(rs.getInt(1)).isEqualTo(1); - rs.close(); - stat.executeUpdate("insert into t1 (v) values ('blue');"); - rs = stat.getGeneratedKeys(); - assertThat(rs.next()).isTrue(); - assertThat(rs.getInt(1)).isEqualTo(2); - rs.close(); - // closing one statement shouldn't close shared db metadata object. - stat.close(); - Statement stat2 = conn.createStatement(); - rs = stat2.getGeneratedKeys(); - assertThat(rs).isNotNull(); - rs.close(); - stat2.close(); + assertThatExceptionOfType(SQLFeatureNotSupportedException.class) + .isThrownBy(() -> stat.getGeneratedKeys()); } @Test