Skip to content

Commit

Permalink
fix(jdbc): remove support for Statement#getGeneratedKeys
Browse files Browse the repository at this point in the history
SQLite does not provide a good way to retrieve generated keys in a reliable way

BREAKING-CHANGE: getGeneratedKeys is not supported anymore

Closes: #329
  • Loading branch information
gotson committed Aug 25, 2023
1 parent c4ddd1e commit 712a8a5
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 35 deletions.
12 changes: 5 additions & 7 deletions src/main/java/org/sqlite/core/CoreDatabaseMetaData.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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 */
Expand Down Expand Up @@ -122,9 +124,6 @@ public synchronized void close() throws SQLException {
if (getColumnPrivileges != null) {
getColumnPrivileges.close();
}
if (getGeneratedKeys != null) {
getGeneratedKeys.close();
}

getTables = null;
getTableTypes = null;
Expand All @@ -143,7 +142,6 @@ public synchronized void close() throws SQLException {
getBestRowIdentifier = null;
getVersionColumns = null;
getColumnPrivileges = null;
getGeneratedKeys = null;
} finally {
conn = null;
}
Expand Down
13 changes: 5 additions & 8 deletions src/main/java/org/sqlite/jdbc3/JDBC3DatabaseMetaData.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() */
Expand Down Expand Up @@ -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. */
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/sqlite/jdbc3/JDBC3Statement.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a
* href=https://www.sqlite.org/lang_returning.html>RETURNING</a> clause instead.
*
* @see java.sql.Statement#getGeneratedKeys()
*/
public ResultSet getGeneratedKeys() throws SQLException {
return conn.getSQLiteDatabaseMetaData().getGeneratedKeys();
throw unsupported();
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/test/java/org/sqlite/DBMetaDataTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 2 additions & 16 deletions src/test/java/org/sqlite/StatementTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

2 comments on commit 712a8a5

@gwenn
Copy link

@gwenn gwenn commented on 712a8a5 Aug 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have been able to support getGeneratedKeys with RETURNING like here.

@dkfellows
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well thank you for breaking code in uses of the library which didn't share connections between threads.

Please sign in to comment.