#794 Support execution of SET TRANSACTION, COMMIT and ROLLBACK statem…

…ents

devdoc/jdp/jdp-2024-01-explicit-support-for-transaction-statements.adoc

@@ -2,8 +2,8 @@
 
 == Status
 
-* Draft
-* Proposed for: Jaybird 6
+* Published: 2024-03-30
+* Implemented in: Jaybird 6
 
 == Type
 
@@ -30,7 +30,7 @@ ____
 In Jaybird 5 and earlier, attempts to use these transaction control statements have various problems:
 
 `SET TRANSACTION`::
-Fails with an error because the statement is executed with a valid transaction handle, while Firebird expects it to be executed without a transaction handle (that is, handle `-1`).
+Fails with an error because the statement is executed with a valid transaction handle, while Firebird expects it to be executed without a transaction handle (that is, handle `0`).
 `COMMIT`::
 Without `RETAIN`:::
 This will seem to work, but subsequent use of the connection will fail as Jaybird assumes it still has a valid transaction.
@@ -55,6 +55,7 @@ There are three possible directions to handle this:
 . Explicitly forbid the statements that don't work (i.e. `SET TRANSACTION`, and `COMMIT`/`ROLLBACK` without `RETAIN`), and leave the others as-is.
 . Explicitly support these statements by performing the same operations which would normally be applied through the API (notify listeners, etc.).
 . Make it configurable, that is, default to disallowing `SET TRANSACTION`, and `COMMIT`/`ROLLBACK` without `RETAIN`, and provide a configuration property to allow their use.
+
 The other transaction statements listed above will always work, but will do no additional work (like trying to add or remove savepoints from the list maintained by the connection implementation).
 
 (Other combinations of "`leave as-is`"/support/forbid would be possible as well, but those listed seem to make the most sense.)
@@ -75,15 +76,15 @@ Given the methods are called, listeners will be notified in the same way as when
 
 When set to `false` (the default), any attempt to execute `SET TRANSACTION`, `COMMIT`, or `ROLLBACK` (without `RETAIN` or `RELEASE SAVEPOINT`) will result in an exception, regardless of auto-commit status and/or status of the current transaction.
 
-With regard to `CallableStatement` (`prepareCall`), nothing will be changed, and execution will continue to fail, because the implementation currently unconditionally converts anything it can't parse to -- for example -- `EXECUTE PROCEDURE COMMIT`, which will obviously fail.
+With regard to `prepareCall`, the implementation will explicitly reject attempts to prepare these statements.
+Although the current implementation of `CallableStatement` would fail anyway, we thought it prudent to disallow it explicitly, in case future changes to `CallableStatement` would accidentally allow it.
 
 Execution of the transaction management statements with any form of `executeQuery` will fail with an exception *before* executing the statement (or equivalent) on the server, because these statements do not produce a result set.
 
 Using `Statement.addBatch(String)` with `COMMIT`, `ROLLBACK` or `SET TRANSACTION` will not be allowed and throw an exception.
-Supporting these statements in batch execution requires careful handling of statement/transaction start and completion boundaries after `COMMIT`/`ROLLBACK` and for `SET TRANSACTION` as first statement, or after `COMMIT`/`ROLLBACK`.
-If there is demand for this, we can consider implementing this at a later time.
 
 It will not be possible to call `PreparedStatement.addBatch()` on a prepared statement with `COMMIT`, `ROLLBACK` or `SET TRANSACTION`, only execution with `execute`, `executeUpdate` or `executeLargeUpdate` is allowed.
+The methods `PreparedStatement.executeBatch()` and `PreparedStatement.executeLargeBatch()` methods will not throw an exception, but effectively do nothing (as the batch will always be empty).
 
 The behaviour of other transaction management statements will be left untouched (at least by this JDP), even when they don't make sense, like using `SAVEPOINT`, `ROLLBACK TO SAVEPOINT` or `RELEASE SAVEPOINT` in an auto-commit transaction.
 
@@ -115,6 +116,18 @@ Invoking `java.sql.Connection#commit()` and `java.sql.Connection#rollback()` ins
 Allow execution through `CallableStatement`::
 Using `CallableStatement` for this doesn't make much sense, and the current implementation doesn't allow for this (in practice, it can only handle call-escapes and execute procedure).
 It would need to be heavily refactored to address this one edge-case.
++
+Instead, we explicitly disallowed preparing these statements with `prepareCall` to prevent future changes to the `CallableStatement` implementation from partially or incorrectly supporting these statements.
+
+Supporting `Statement.addBatch(String)`::
+Supporting these statements in batch execution requires careful handling of statement/transaction start and completion boundaries after `COMMIT`/`ROLLBACK` and for `SET TRANSACTION` as first statement, or after `COMMIT`/`ROLLBACK`.
++
+If there is demand for this, we can consider implementing this at a later time.
+
+Supporting `PreparedStatement.addBatch()`::
+Executing prepared statement batches of these statements does not make sense.
+In the case of `COMMIT` and `ROLLBACK`, the second and subsequent entries in the batch would effectively do nothing.
+In the case of `SET TRANSACTION`, the second and subsequent entries would fail with an error as the connection would have an active transaction from the first entry.
 
 == Consequences
 
@@ -130,12 +143,12 @@ When `allowTxStmts` is set to `true`, the following is supported:
 
 === `COMMIT`
 
-Attempts to execute `COMMIT [WORK]` will unconditionally call `commit()` on the connection, and exhibit the same behaviour for commit required by the JDBC specification.
+Attempts to execute `COMMIT [WORK]` will call `commit()` on the connection, and exhibit the same behaviour for commit required by the JDBC specification.
 This means that if auto-commit is enabled, or the connection is participating in a distributed transaction, an exception is thrown that explicit commit is not allowed.
 
 === `ROLLBACK`
 
-Attempts to execute `ROLLBACK [WORK]` will unconditionally call `rollback()` on the connection, and exhibit the same behaviour for rollback required by the JDBC specification.
+Attempts to execute `ROLLBACK [WORK]` will call `rollback()` on the connection, and exhibit the same behaviour for rollback required by the JDBC specification.
 This means that if auto-commit is enabled, or the connection is participating in a distributed transaction, an exception is thrown that explicit rollback is not allowed.
 
 === Limitations

jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java

@@ -225,14 +225,35 @@ public void cancelOperation(int kind) throws SQLException {
     public JnaTransaction startTransaction(final TransactionParameterBuffer tpb) throws SQLException {
         try {
             checkConnected();
-            final IntByReference transactionHandle = new IntByReference(0);
-            final byte[] tpbArray = tpb.toBytesWithType();
+            var transactionHandle = new IntByReference(0);
+            byte[] tpbArray = tpb.toBytesWithType();
             try (LockCloseable ignored = withLock()) {
                 clientLibrary.isc_start_transaction(statusVector, transactionHandle, (short) 1, handle,
                         (short) tpbArray.length, tpbArray);
                 processStatusVector();
 
-                final JnaTransaction transaction = new JnaTransaction(this, transactionHandle, TransactionState.ACTIVE);
+                var transaction = new JnaTransaction(this, transactionHandle, TransactionState.ACTIVE);
+                transactionAdded(transaction);
+                return transaction;
+            }
+        } catch (SQLException e) {
+            exceptionListenerDispatcher.errorOccurred(e);
+            throw e;
+        }
+    }
+
+    @Override
+    public FbTransaction startTransaction(String statementText) throws SQLException {
+        try {
+            checkConnected();
+            var transactionHandle = new IntByReference(0);
+            byte[] statementArray = getEncoding().encodeToCharset(statementText);
+            try (LockCloseable ignored = withLock()) {
+                clientLibrary.isc_dsql_execute_immediate(statusVector, handle, transactionHandle,
+                        (short) statementArray.length, statementArray, getConnectionDialect(), null);
+                processStatusVector();
+
+                var transaction = new JnaTransaction(this, transactionHandle, TransactionState.ACTIVE);
                 transactionAdded(transaction);
                 return transaction;
             }

src/docs/asciidoc/release_notes.adoc

@@ -849,6 +849,50 @@ This method will read until `len` bytes have been read, and only return less tha
 The documentation of method `FbBlob.putSegment(byte[])` contradicted itself, by requiring implementations to batch larger arrays, but also requiring them to throw an exception for larger arrays, and the actual implementations provided by Jaybird threw an exception.
 This contradiction has been removed, and the implementations will now send arrays longer than the maximum segment size to the server in multiple _put_ requests.
 
+[#allow-tx-stmts]
+=== Support for executing transaction management statements
+
+In Jaybird 5 and earlier, it was not possible to execute the transaction management statements `COMMIT`, `ROLLBACK` (without retain or savepoints) and `SET TRANSACTION`.
+For `COMMIT` and `ROLLBACK` it would seem to work, but subsequent use of the connection would then break because the connection assumed it still had an active transaction.
+
+Formally, the JDBC specification says -- paraphrased -- that if something can be done through the JDBC API, that those API methods should be used, and not using equivalent statements.
+However, from a perspective of flexibility, and for example for executing scripts, it can be useful to be able to execute those statements.
+
+Jaybird now optionally allows you to execute `COMMIT [WORK]`, `ROLLBACK [WORK]` and `SET TRANSACTION [...]`.
+By default, Jaybird 6 explicitly rejects attempts to execute those statements, instead of the half-working/half-broken situation of previous versions.
+
+To allow execution of `COMMIT [WORK]`, `ROLLBACK [WORK]` and `SET TRANSACTION [...]`, the connection property `allowTxStmts` needs to be set to `true`.
+This can be done using a JDBC connection property `allowTxStmts`, or `setAllowTxStmts(boolean)` on `DataSource` instances.
+
+[NOTE]
+====
+Just because you can, doesn't mean you should use this.
+For code solutions, you should use the normal methods in the JDBC API whenever possible.
+Only use this solution for scripts, or in case it is cumbersome or not possible to access the Jaybird extensions to the JDBC API to control the transaction configuration.
+====
+
+In the implementation, the use of `COMMIT` and `ROLLBACK` will not be executed as such, but instead call `Connection.commit()` and `Connection.rollback()`.
+The `SET TRANSACTION` statement -- if allowed -- is executed with execute immediate, and not through a statement handle.
+
+Enabling this feature can also make it easier to use the table reservation feature, compared to `FirebirdConnection.setTransactionParameters(TransactionParameterBuffer)` or `FirebirdConnection.setTransactionParameters(int TransactionParameterBuffer)`, which requires access to the Jaybird API interfaces.
+
+[NOTE]
+====
+Contrary to its name, and the SQL standard behaviour, Firebird's `SET TRANSACTION` immediately *starts* a transaction.
+====
+
+This feature has the following limitations:
+
+* Transaction management statements cannot be executed when auto-commit is enabled, or if the connection is participating in a distributed transaction.
+This is the same behaviour as implemented for `Connection.commit()` and `Connection.rollback()`.
+* Executing `COMMIT` or `ROLLBACK` -- when auto-commit is disabled -- is silently ignored if there is no active transaction.
+This is the same behaviour as implemented for `Connection.commit()` and `Connection.rollback()`.
+* `SET TRANSACTION` cannot be executed if there is an active transaction.
+In other words, you will need to call `Connection.commit()` or execute `COMMIT` (or roll back) before you can start a new transaction this way.
+* Transaction management statements are not supported by `Statement.addBatch(String)`, `PreparedStatement.addBatch()`, and `Connection.prepareCall(...)`.
+
+For more information, see also https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2024-01-explicit-support-for-transaction-statements.adoc[jdp-2024-01: Explicit support for transaction statements^].
+
 // TODO add major changes
 
 [#other-fixes-and-changes]
@@ -1058,6 +1102,36 @@ Though it can parse it, the resulting value will not include fractional seconds.
 * `getDate(..., Calendar)`  on a `CHAR`/`VARCHAR`/`BLOB SUB_TYPE TEXT` field will now use the `Calendar` to rebase the date, this can result in an off-by-one difference in the date compared to previous versions (depending on the time zone set on the `Calendar`)
 * The `TypeConversionException` thrown by `getDate(...)`, `getTime(...)` and `getTimestamp(...)` on unsupported types may now report `java.time.LocalDate`, `java.time.LocalTime` or `java.time.LocalDateTime` as the type in its error message instead of `java.sql.Date`, `java.sql.Time`, or `java.sql.Timestamp`
 
+[#compat-allow-tx-stmts]
+=== Execution of `COMMIT` and `ROLLBACK` rejected by default
+
+Attempts to prepare or execute `COMMIT` or `ROLLBACK` (without retain) will now fail by default.
+In previous versions, executing these statements would work, but leave the connection in an unusable state.
+The exact error will -- generally -- be one of the following:
+
+[horizontal]
+`337248313`::
+"`__Execution of COMMIT statement is not allowed, use Connection.commit(), or set connection property allowTxStmts to true__`"
+`337248314`::
+"`__Execution of ROLLBACK statement is not allowed, use Connection.rollback(), or set connection property allowTxStmts to true__`"
+`337248319`::
+"`__Using addBatch with a transaction management statement is not supported__`"
+`337248320`::
+"`__Using prepareCall with a transaction management statement is not supported__`"
+
+In the case of the `execute`, `executeUpdate` or `executeLargeUpdate` methods of `Statement`, or the `prepareStatement` methods of `Connection`, this can be resolved by allowing the execution with connection property `allowTxStmts` set to `true`.
+
+In the case of `Statement.executeQuery(String)` and `PreparedStatement.executeQuery()`, you will need to switch to using one of the other `execute`, `executeUpdate` or `executeLargeUpdate` methods.
+
+It is not possible to use the `prepareCall` methods of `Connection` with these statements.
+In previous versions of Jaybird, subsequent execution wouldn't work either -- or attempt to execute stored procedures called `COMMIT` or `ROLLBACK`, but it is now rejected early in the `prepareCall` methods of `Connection`.
+Switch to using `prepareStatement`.
+
+Additionally, using `Statement.addBatch(String)` and `PreparedStatement.addBatch()` will not work with these statements.
+Switch to using one of the normal execute methods.
+
+See also <<allow-tx-stmts>>.
+
 // TODO Document compatibility issues
 
 [#removal-of-classes-packages-and-methods-without-deprecation]
@@ -1487,6 +1561,9 @@ The following methods will be removed in Jaybird 7:
 It may get removed in Jaybird 7 or later.
 ** `getSupportedProtocols` -- use `getSupportedProtocolList()`.
 It may get removed in Jaybird 7 or later.
+* `GDSHelper` (internal API)
+** `startTransaction(TransactionParameterBuffer)` -- use `FbDatabase.startTransaction(TransactionParameterBuffer)` followed by `GDSHelper.setCurrentTransaction(FbTransaction)`
+It will be removed in Jaybird 7.
 * `FirebirdStatement`
 ** `getCurrentResultSet()` -- use `getResultSet()`.
 Will be removed in Jaybird 7.

src/main/org/firebirdsql/ds/AbstractConnectionPropertiesDataSource.java

@@ -456,6 +456,16 @@ public void setUseCatalogAsPackage(boolean useCatalogAsPackage) {
         FirebirdConnectionProperties.super.setUseCatalogAsPackage(useCatalogAsPackage);
     }
 
+    @Override
+    public boolean isAllowTxStmts() {
+        return FirebirdConnectionProperties.super.isAllowTxStmts();
+    }
+
+    @Override
+    public void setAllowTxStmts(boolean allowTxStmts) {
+        FirebirdConnectionProperties.super.setAllowTxStmts(allowTxStmts);
+    }
+
     @SuppressWarnings("deprecation")
     @Deprecated(since = "5")
     @Override

src/main/org/firebirdsql/gds/JaybirdErrorCodes.java

@@ -109,6 +109,15 @@ public interface JaybirdErrorCodes {
     int jb_localTransactionActive   = 337248309;
     int jb_invalidFetchSize         = 337248310;
     int jb_operationNotCancellable  = 337248311;
+    int jb_executeQueryWithTxStmt   = 337248312;
+    int jb_commitStatementNotAllowed                     = 337248313;
+    int jb_rollbackStatementNotAllowed                   = 337248314;
+    int jb_setTransactionStatementNotAllowed             = 337248315;
+    int jb_setTransactionNotAllowedInAutoCommit          = 337248316;
+    int jb_setTransactionNotAllowedActiveTx              = 337248317;
+    int jb_statementNotAssociatedWithConnection          = 337248318;
+    int jb_addBatchWithTxStmt       = 337248319;
+    int jb_prepareCallWithTxStmt    = 337248320;
 
     @SuppressWarnings("unused")
     int jb_range_end                = 337264639;

src/main/org/firebirdsql/gds/impl/GDSHelper.java

@@ -147,11 +147,15 @@ public FbBlob createBlob(BlobConfig blobConfig) throws SQLException {
         return blob;
     }
 
+    /**
+     * @deprecated Will be removed in Jaybird 7, use {@link FbDatabase#startTransaction(TransactionParameterBuffer)} and
+     * {@link #setCurrentTransaction(FbTransaction)}
+     */
+    @Deprecated(forRemoval = true, since = "6")
     public FbTransaction startTransaction(TransactionParameterBuffer tpb) throws SQLException {
-        FbTransaction transaction = database.startTransaction(tpb);
-        setCurrentTransaction(transaction);
-
-        return transaction;
+        FbTransaction newTx = database.startTransaction(tpb);
+        setCurrentTransaction(newTx);
+        return newTx;
     }
 
     public void detachDatabase() throws SQLException {

src/main/org/firebirdsql/gds/ng/FbDatabase.java

@@ -95,6 +95,18 @@ public interface FbDatabase extends FbAttachment {
      */
     FbTransaction startTransaction(TransactionParameterBuffer tpb) throws SQLException;
 
+    /**
+     * Creates and starts a transaction using a SQL statement
+     *
+     * @param statementText
+     *         statement which starts a transaction
+     * @return FbTransaction
+     * @throws SQLException
+     *         for database access error
+     * @since 6
+     */
+    FbTransaction startTransaction(String statementText) throws SQLException;
+
     /**
      * Reconnects a prepared transaction.
      * <p>

src/main/org/firebirdsql/gds/ng/wire/version10/V10Database.java

@@ -301,6 +301,18 @@ public final FbWireTransaction startTransaction(TransactionParameterBuffer tpb)
         }
     }
 
+    @Override
+    public FbTransaction startTransaction(String statementText) throws SQLException {
+        try (LockCloseable ignored = withLock()) {
+            checkAttached();
+            sendExecuteImmediate(statementText, null);
+            return receiveTransactionResponse(TransactionState.ACTIVE);
+        } catch (SQLException ex) {
+            exceptionListenerDispatcher.errorOccurred(ex);
+            throw ex;
+        }
+    }
+
     private void sendStartTransaction(TransactionParameterBuffer tpb) throws SQLException {
         try {
             final XdrOutputStream xdrOut = getXdrOut();