diff --git a/src/docs/asciidoc/release_notes.adoc b/src/docs/asciidoc/release_notes.adoc
index ba925f1aa..50c7f1099 100644
--- a/src/docs/asciidoc/release_notes.adoc
+++ b/src/docs/asciidoc/release_notes.adoc
@@ -36,6 +36,11 @@ The following has been changed or fixed since Jaybird 5.0.3:
+
As part of this change, the constructor `AbstractWireOperations` now explicitly requires the `connection` and `defaultWarningMessageCallback` to be non-``null`` and throws a `NullPointerException` otherwise.
This may affect custom protocol implementations extending `AbstractWireOperations` and/or calling `ProtocolDescriptor#createWireOperations(WireConnection, WarningMessageCallback)`: make sure you don't pass `null`.
+* Fixed: FBRowUpdater incorrectly considers result set with only partial PK updatable -- backported from Jaybird 6 (https://github.com/FirebirdSQL/jaybird/issues/780[#780])
++
+This change also improves performance of `updateRow()`, `insertRow()`, `deleteRow()` and `refreshRow()`.
+The best row identifier or `RDB$DB_KEY` were detected _each time_ when calling `updateRow()`, `insertRow()`, `deleteRow()`, or `refreshRow()`.
+This has been improved so this detection is done once, and in a way that non-updatable result sets can now be downgraded to `CONCUR_READ_ONLY` instead of throwing an exception when performing the modification.
* ...
[#jaybird-5-0-3-changelog]
diff --git a/src/main/org/firebirdsql/jdbc/FBRowUpdater.java b/src/main/org/firebirdsql/jdbc/FBRowUpdater.java
index aed724fe0..6388501fc 100644
--- a/src/main/org/firebirdsql/jdbc/FBRowUpdater.java
+++ b/src/main/org/firebirdsql/jdbc/FBRowUpdater.java
@@ -69,7 +69,6 @@ final class FBRowUpdater implements FirebirdRowUpdater {
private static final int PARAMETER_DBKEY = 2;
private static final byte[][] EMPTY_2D_BYTES = new byte[0][];
- private final FBConnection connection;
private final GDSHelper gdsHelper;
private final RowDescriptor rowDescriptor;
private final FBField[] fields;
@@ -81,6 +80,7 @@ final class FBRowUpdater implements FirebirdRowUpdater {
private RowValue oldRow;
private RowValue insertRow;
private boolean[] updatedFlags;
+ private final int[] parameterMask;
private String tableName;
@@ -92,19 +92,27 @@ final class FBRowUpdater implements FirebirdRowUpdater {
FBRowUpdater(FBConnection connection, RowDescriptor rowDescriptor, boolean cached,
FBObjectListener.ResultSetListener rsListener) throws SQLException {
- this.rsListener = rsListener;
+ // find the table name (there can be only one table per result set)
+ for (FieldDescriptor fieldDescriptor : rowDescriptor) {
+ if (tableName == null) {
+ tableName = fieldDescriptor.getOriginalTableName();
+ } else if (!tableName.equals(fieldDescriptor.getOriginalTableName())) {
+ throw new FBResultSetNotUpdatableException(
+ "Underlying result set references at least two relations: " +
+ tableName + " and " + fieldDescriptor.getOriginalTableName() + ".");
+ }
+ }
+ parameterMask = getParameterMask(tableName, rowDescriptor, connection.getMetaData());
- this.connection = connection;
+ this.rsListener = rsListener;
gdsHelper = connection.getGDSHelper();
-
- this.rowDescriptor = rowDescriptor;
- fields = new FBField[rowDescriptor.getCount()];
-
quoteStrategy = QuoteStrategy.forDialect(gdsHelper.getDialect());
+ this.rowDescriptor = rowDescriptor;
newRow = rowDescriptor.createDefaultFieldValues();
updatedFlags = new boolean[rowDescriptor.getCount()];
+ fields = new FBField[rowDescriptor.getCount()];
for (int i = 0; i < rowDescriptor.getCount(); i++) {
final int fieldPos = i;
@@ -134,17 +142,6 @@ public void setFieldData(byte[] data) {
fields[i] = FBField.createField(rowDescriptor.getFieldDescriptor(i), dataProvider, gdsHelper, cached);
}
-
- // find the table name (there can be only one table per result set)
- for (FieldDescriptor fieldDescriptor : rowDescriptor) {
- if (tableName == null) {
- tableName = fieldDescriptor.getOriginalTableName();
- } else if (!tableName.equals(fieldDescriptor.getOriginalTableName())) {
- throw new FBResultSetNotUpdatableException(
- "Underlying result set references at least two relations: " +
- tableName + " and " + fieldDescriptor.getOriginalTableName() + ".");
- }
- }
}
private void notifyExecutionStarted() throws SQLException {
@@ -222,40 +219,39 @@ public FBField getField(int fieldPosition) {
*
* @return array of booleans that represent parameter mask.
*/
- private int[] getParameterMask() throws SQLException {
+ private static int[] getParameterMask(String tableName, RowDescriptor rowDescriptor, DatabaseMetaData dbmd)
+ throws SQLException {
// loop through the "best row identifiers" and set appropriate flags.
- FBDatabaseMetaData metaData = (FBDatabaseMetaData) connection.getMetaData();
-
- try (ResultSet bestRowIdentifier = metaData.getBestRowIdentifier("", "", tableName,
+ try (ResultSet bestRowIdentifier = dbmd.getBestRowIdentifier("", "", tableName,
DatabaseMetaData.bestRowTransaction, true)) {
int[] result = new int[rowDescriptor.getCount()];
boolean hasParams = false;
while (bestRowIdentifier.next()) {
String columnName = bestRowIdentifier.getString(2);
+ if (columnName == null) continue;
- if (columnName == null)
- continue;
-
+ boolean found = false;
for (int i = 0; i < rowDescriptor.getCount(); i++) {
// special handling for the RDB$DB_KEY columns that must be
// selected as RDB$DB_KEY, but in XSQLVAR are represented
// as DB_KEY
if ("RDB$DB_KEY".equals(columnName) && rowDescriptor.getFieldDescriptor(i).isDbKey()) {
result[i] = PARAMETER_DBKEY;
- hasParams = true;
+ found = true;
} else if (columnName.equals(rowDescriptor.getFieldDescriptor(i).getOriginalName())) {
result[i] = PARAMETER_USED;
- hasParams = true;
+ found = true;
}
}
// if we did not find a column from the best row identifier
// in our result set, throw an exception, since we cannot
// reliably identify the row.
- if (!hasParams)
+ if (!found) {
throw new FBResultSetNotUpdatableException(
- "Underlying result set does not contain all columns " +
- "that form 'best row identifier'.");
+ "Underlying result set does not contain all columns that form 'best row identifier'.");
+ }
+ hasParams = true;
}
if (!hasParams)
@@ -266,7 +262,7 @@ private int[] getParameterMask() throws SQLException {
}
}
- private void appendWhereClause(StringBuilder sb, int[] parameterMask) {
+ private void appendWhereClause(StringBuilder sb) {
sb.append("WHERE ");
// handle the RDB$DB_KEY case first
@@ -299,7 +295,7 @@ private void appendWhereClause(StringBuilder sb, int[] parameterMask) {
}
}
- private String buildUpdateStatement(int[] parameterMask) {
+ private String buildUpdateStatement() {
StringBuilder sb = new StringBuilder("UPDATE ");
quoteStrategy.appendQuoted(tableName, sb)
.append("\nSET\n");
@@ -319,15 +315,15 @@ private String buildUpdateStatement(int[] parameterMask) {
}
sb.append('\n');
- appendWhereClause(sb, parameterMask);
+ appendWhereClause(sb);
return sb.toString();
}
- private String buildDeleteStatement(int[] parameterMask) {
+ private String buildDeleteStatement() {
StringBuilder sb = new StringBuilder("DELETE FROM ");
quoteStrategy.appendQuoted(tableName, sb).append('\n');
- appendWhereClause(sb, parameterMask);
+ appendWhereClause(sb);
return sb.toString();
}
@@ -360,7 +356,7 @@ private String buildInsertStatement() {
return sb.toString();
}
- private String buildSelectStatement(int[] parameterMask) {
+ private String buildSelectStatement() {
StringBuilder columns = new StringBuilder();
boolean first = true;
@@ -382,7 +378,7 @@ private String buildSelectStatement(int[] parameterMask) {
sb.append(columns).append('\n')
.append("FROM ");
quoteStrategy.appendQuoted(tableName, sb).append('\n');
- appendWhereClause(sb, parameterMask);
+ appendWhereClause(sb);
return sb.toString();
}
@@ -487,16 +483,14 @@ private void executeStatement(int statementType, FbStatement stmt) throws SQLExc
((FBFlushableField) fields[i]).flushCachedData();
}
- int[] parameterMask = getParameterMask();
-
String sql;
switch (statementType) {
case UPDATE_STATEMENT_TYPE:
- sql = buildUpdateStatement(parameterMask);
+ sql = buildUpdateStatement();
break;
case DELETE_STATEMENT_TYPE:
- sql = buildDeleteStatement(parameterMask);
+ sql = buildDeleteStatement();
break;
case INSERT_STATEMENT_TYPE:
@@ -504,7 +498,7 @@ private void executeStatement(int statementType, FbStatement stmt) throws SQLExc
break;
case SELECT_STATEMENT_TYPE:
- sql = buildSelectStatement(parameterMask);
+ sql = buildSelectStatement();
break;
default:
diff --git a/src/test/org/firebirdsql/jdbc/FBResultSetTest.java b/src/test/org/firebirdsql/jdbc/FBResultSetTest.java
index d51138ba1..29eab610f 100644
--- a/src/test/org/firebirdsql/jdbc/FBResultSetTest.java
+++ b/src/test/org/firebirdsql/jdbc/FBResultSetTest.java
@@ -87,6 +87,14 @@ class FBResultSetTest {
" \"CamelStr\" VARCHAR(255)" +
")";
+ private static final String CREATE_WITH_COMPOSITE_PK =
+ "create table WITH_COMPOSITE_PK (\n" +
+ " ID1 integer not null,\n" +
+ " ID2 integer not null,\n" +
+ " VAL varchar(50),\n" +
+ " constraint PK_WITH_COMPOSITE_PK primary key (ID1, ID2)\n" +
+ ")";
+
private static final String CREATE_VIEW_STATEMENT =
"CREATE VIEW test_empty_string_view(marker, id, empty_char) " +
" AS " +
@@ -705,6 +713,50 @@ void testUpdatableStatementResultSetDowngradeToReadOnlyWhenQueryNotUpdatable(
}
}
+ /**
+ * Tests if a result set that doesn't contain all PK columns (but only a prefix) will be downgraded to read-only.
+ *
+ * Rationale: a previous implementation was satisfied if at least the first PK column was found
+ *
+ */
+ @ParameterizedTest
+ @MethodSource("scrollableCursorPropertyValues")
+ void testUpdatableStatementPrefixPK_downgradeToReadOnly(String scrollableCursorPropertyValue) throws Exception {
+ try (Connection connection = createConnection(scrollableCursorPropertyValue)) {
+ executeCreateTable(connection, CREATE_WITH_COMPOSITE_PK);
+ try (var stmt = connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_UPDATABLE);
+ var rs = stmt.executeQuery("select id1, val from WITH_COMPOSITE_PK")) {
+ assertThat(stmt.getWarnings(), allOf(
+ notNullValue(),
+ fbMessageStartsWith(JaybirdErrorCodes.jb_concurrencyResetReadOnlyReasonNotUpdatable)));
+
+ assertEquals(CONCUR_READ_ONLY, rs.getConcurrency(), "Expected downgrade to CONCUR_READ_ONLY");
+ }
+ }
+ }
+
+ /**
+ * Tests if a result set that doesn't contain all PK columns (but only a suffix) will be downgraded to read-only.
+ *
+ * Rationale: a previous implementation was satisfied if at least the first PK column was found
+ *
+ */
+ @ParameterizedTest
+ @MethodSource("scrollableCursorPropertyValues")
+ void testUpdatableStatementSuffixPK_downgradeToReadOnly(String scrollableCursorPropertyValue) throws Exception {
+ try (Connection connection = createConnection(scrollableCursorPropertyValue)) {
+ executeCreateTable(connection, CREATE_WITH_COMPOSITE_PK);
+ try (var stmt = connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_UPDATABLE);
+ var rs = stmt.executeQuery("select id2, val from WITH_COMPOSITE_PK")) {
+ assertThat(stmt.getWarnings(), allOf(
+ notNullValue(),
+ fbMessageStartsWith(JaybirdErrorCodes.jb_concurrencyResetReadOnlyReasonNotUpdatable)));
+
+ assertEquals(CONCUR_READ_ONLY, rs.getConcurrency(), "Expected downgrade to CONCUR_READ_ONLY");
+ }
+ }
+ }
+
@Test
void testGetExecutionPlan() throws Exception {
try (Connection connection = getConnectionViaDriverManager()) {