[pdo_firebird] Transaction management optimization (#12741)

take2 of #12657

## About Firebird transaction

Firebird is a full transactional database, so the DB itself does not support autocommit mode. (Strictly, there is an autocommit mode, but it is a different concept from the "autocommit" that we are used to with MySQL and others.)

Therefore, a transaction must have started before any operation is performed, and autocommit mode had to be emulated in PHP.

I made sure that a transaction always exists when in autocommit mode. Since the `in_transacntion` function does not work as expected, I have introduced `H->in_manually_txn` to determine whether a transaction is being manually manipulated.

## There are two types of commit/rollback

(I'm not talking about two-phase commit. This change does not take into account two-phase commit.)

There are `isc_commit_retaining` which starts a transaction again in the same context immediately after committing, and `isc_commit_transaction` which closes the transaction as is.

Similarly, there are two types of rollback.

-----------

Due to the default value of the transaction isolation level, autocommit mode may obtain unintended results.
Regarding this, it would be too large to include support for transaction isolation levels in this PR, so I will leave it as is for now.

Author: SakiTakamachi

ext/pdo/tests/pdo_017.phpt

@@ -8,7 +8,6 @@ $dir = getenv('REDIR_TEST_DIR');
 if (false == $dir) die('skip no driver');
 require_once $dir . 'pdo_test.inc';
 PDOTest::skip();
-if (str_starts_with(getenv('PDOTEST_DSN'), "firebird")) die('xfail firebird driver does not behave as expected');
 
 $db = PDOTest::factory();
 try {

ext/pdo_firebird/firebird_driver.c

@@ -33,6 +33,7 @@
 
 static int php_firebird_alloc_prepare_stmt(pdo_dbh_t*, const zend_string*, XSQLDA*, isc_stmt_handle*,
 	HashTable*);
+static bool php_firebird_rollback_transaction(pdo_dbh_t *dbh);
 
 const char CHR_LETTER = 1;
 const char CHR_DIGIT = 2;
@@ -526,17 +527,14 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (dbh->in_txn) {
+	if (H->tr) {
 		if (dbh->auto_commit) {
-			if (isc_commit_transaction(H->isc_status, &H->tr)) {
-				php_firebird_error(dbh);
-			}
+			php_firebird_commit_transaction(dbh, /* release */ false);
 		} else {
-			if (isc_rollback_transaction(H->isc_status, &H->tr)) {
-				php_firebird_error(dbh);
-			}
+			php_firebird_rollback_transaction(dbh);
 		}
 	}
+	H->in_manually_txn = 0;
 
 	if (isc_detach_database(H->isc_status, &H->db)) {
 		php_firebird_error(dbh);
@@ -702,9 +700,10 @@ static zend_long firebird_handle_doer(pdo_dbh_t *dbh, const zend_string *sql) /*
 		}
 	}
 
-	/* commit if we're in auto_commit mode */
-	if (dbh->auto_commit && isc_commit_retaining(H->isc_status, &H->tr)) {
-		php_firebird_error(dbh);
+	if (dbh->auto_commit && !H->in_manually_txn) {
+		if (!php_firebird_commit_transaction(dbh, /* retain */ true)) {
+			ret = -1;
+		}
 	}
 
 free_statement:
@@ -756,8 +755,8 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
 }
 /* }}} */
 
-/* called by PDO to start a transaction */
-static bool firebird_handle_begin(pdo_dbh_t *dbh) /* {{{ */
+/* php_firebird_begin_transaction */
+static bool php_firebird_begin_transaction(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 	char tpb[8] = { isc_tpb_version3 }, *ptpb = tpb+1;
@@ -809,12 +808,84 @@ static bool firebird_handle_begin(pdo_dbh_t *dbh) /* {{{ */
 }
 /* }}} */
 
+/* called by PDO to start a transaction */
+static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+
+	/**
+	 * If in autocommit mode and in transaction, we will need to close the transaction once.
+	 */
+	if (dbh->auto_commit && H->tr) {
+		if (!php_firebird_commit_transaction(dbh, /* release */ false)) {
+			return false;
+		}
+	}
+
+	if (!php_firebird_begin_transaction(dbh)) {
+		return false;
+	}
+	H->in_manually_txn = 1;
+	return true;
+}
+/* }}} */
+
+/* php_firebird_commit_transaction */
+bool php_firebird_commit_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+
+	/**
+	 * `retaining` keeps the transaction open without closing it.
+	 *
+	 * firebird needs to always have a transaction open to emulate autocommit mode,
+	 * and in autocommit mode it keeps the transaction open.
+	 *
+	 * Same as close and then begin again, but use retain to save overhead.
+	 */
+	if (retain) {
+		if (isc_commit_retaining(H->isc_status, &H->tr)) {
+			php_firebird_error(dbh);
+			return false;
+		}
+	} else {
+		if (isc_commit_transaction(H->isc_status, &H->tr)) {
+			php_firebird_error(dbh);
+			return false;
+		}
+	}
+	return true;
+}
+/* }}} */
+
 /* called by PDO to commit a transaction */
-static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
+static bool firebird_handle_manually_commit(pdo_dbh_t *dbh) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+	if (!php_firebird_commit_transaction(dbh, /*release*/ false)) {
+		return false;
+	}
+
+	/**
+	 * If in autocommit mode, begin the transaction again
+	 * Reopen instead of retain because isolation level may change
+	 */
+	if (dbh->auto_commit) {
+		if (!php_firebird_begin_transaction(dbh)) {
+			return false;
+		}
+	}
+	H->in_manually_txn = 0;
+	return true;
+}
+/* }}} */
+
+/* php_firebird_rollback_transaction */
+static bool php_firebird_rollback_transaction(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (isc_commit_transaction(H->isc_status, &H->tr)) {
+	if (isc_rollback_transaction(H->isc_status, &H->tr)) {
 		php_firebird_error(dbh);
 		return false;
 	}
@@ -823,14 +894,24 @@ static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
 /* }}} */
 
 /* called by PDO to rollback a transaction */
-static bool firebird_handle_rollback(pdo_dbh_t *dbh) /* {{{ */
+static bool firebird_handle_manually_rollback(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (isc_rollback_transaction(H->isc_status, &H->tr)) {
-		php_firebird_error(dbh);
+	if (!php_firebird_rollback_transaction(dbh)) {
 		return false;
 	}
+
+	/**
+	 * If in autocommit mode, begin the transaction again
+	 * Reopen instead of retain because isolation level may change
+	 */
+	if (dbh->auto_commit) {
+		if (!php_firebird_begin_transaction(dbh)) {
+			return false;
+		}
+	}
+	H->in_manually_txn = 0;
 	return true;
 }
 /* }}} */
@@ -848,16 +929,6 @@ static int php_firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sq
 		return 0;
 	}
 
-	/* start a new transaction implicitly if auto_commit is enabled and no transaction is open */
-	if (dbh->auto_commit && !dbh->in_txn) {
-		/* dbh->transaction_flags = PDO_TRANS_READ_UNCOMMITTED; */
-
-		if (!firebird_handle_begin(dbh)) {
-			return 0;
-		}
-		dbh->in_txn = true;
-	}
-
 	/* allocate the statement */
 	if (isc_dsql_allocate_statement(H->isc_status, &H->db, s)) {
 		php_firebird_error(dbh);
@@ -898,21 +969,34 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val
 					return false;
 				}
 
+				if (H->in_manually_txn) {
+					/* change auto commit mode with an open transaction is illegal, because
+						we won't know what to do with it */
+					pdo_raise_impl_error(dbh, NULL, "HY000", "Cannot change autocommit mode while a transaction is already open");
+					return false;
+				}
+
 				/* ignore if the new value equals the old one */
 				if (dbh->auto_commit ^ bval) {
-					if (dbh->in_txn) {
-						if (bval) {
-							/* turning on auto_commit with an open transaction is illegal, because
-							   we won't know what to do with it */
-							const char *msg = "Cannot enable auto-commit while a transaction is already open";
-							php_firebird_error_with_info(dbh, "HY000", strlen("HY000"), msg, strlen(msg));
-							return false;
-						} else {
-							/* close the transaction */
-							if (!firebird_handle_commit(dbh)) {
-								break;
+					if (bval) {
+						/* change to auto commit mode.
+						 * If the transaction is not started, start it.
+						 * However, this is a fallback since such a situation usually does not occur.
+						 */
+						if (!H->tr) {
+							if (!php_firebird_begin_transaction(dbh)) {
+								return false;
+							}
+						}
+					} else {
+						/* change to not auto commit mode.
+						 * close the transaction if exists.
+						 * However, this is a fallback since such a situation usually does not occur.
+						 */
+						if (H->tr) {
+							if (!php_firebird_commit_transaction(dbh, /* release */ false)) {
+								return false;
 							}
-							dbh->in_txn = false;
 						}
 					}
 					dbh->auto_commit = bval;
@@ -1058,22 +1142,35 @@ static void pdo_firebird_fetch_error_func(pdo_dbh_t *dbh, pdo_stmt_t *stmt, zval
 }
 /* }}} */
 
+/* {{{ firebird_in_manually_transaction */
+static bool pdo_firebird_in_manually_transaction(pdo_dbh_t *dbh)
+{
+	/**
+	 * we can tell if a transaction exists now by checking H->tr,
+	 * but which will always be true in autocommit mode.
+	 * So this function checks if there is currently a "manually begun transaction".
+	 */
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+	return H->in_manually_txn;
+}
+/* }}} */
+
 static const struct pdo_dbh_methods firebird_methods = { /* {{{ */
 	firebird_handle_closer,
 	firebird_handle_preparer,
 	firebird_handle_doer,
 	firebird_handle_quoter,
-	firebird_handle_begin,
-	firebird_handle_commit,
-	firebird_handle_rollback,
+	firebird_handle_manually_begin,
+	firebird_handle_manually_commit,
+	firebird_handle_manually_rollback,
 	pdo_firebird_set_attribute,
 	NULL, /* last_id not supported */
 	pdo_firebird_fetch_error_func,
 	pdo_firebird_get_attribute,
 	NULL, /* check_liveness */
 	NULL, /* get driver methods */
 	NULL, /* request shutdown */
-	NULL, /* in transaction, use PDO's internal tracking mechanism */
+	pdo_firebird_in_manually_transaction,
 	NULL /* get gc */
 };
 /* }}} */
@@ -1154,6 +1251,11 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /*
 				"HY000", H->isc_status[1], errmsg);
 	}
 
+	H->in_manually_txn = 0;
+	if (dbh->auto_commit && !H->tr) {
+		ret = php_firebird_begin_transaction(dbh);
+	}
+
 	if (!ret) {
 		firebird_handle_closer(dbh);
 	}

ext/pdo_firebird/firebird_statement.c

@@ -180,8 +180,7 @@ static int pdo_firebird_stmt_execute(pdo_stmt_t *stmt) /* {{{ */
 				;
 		}
 
-		/* commit? */
-		if (stmt->dbh->auto_commit && isc_commit_retaining(H->isc_status, &H->tr)) {
+		if (stmt->dbh->auto_commit && !S->H->in_manually_txn && !php_firebird_commit_transaction(stmt->dbh, /* retain */ true)) {
 			break;
 		}
 

ext/pdo_firebird/php_pdo_firebird_int.h

@@ -66,7 +66,6 @@ typedef struct {
 } pdo_firebird_error_info;
 
 typedef struct {
-
 	/* the result of the last API call */
 	ISC_STATUS isc_status[20];
 
@@ -75,6 +74,7 @@ typedef struct {
 
 	/* the transaction handle */
 	isc_tr_handle tr;
+	bool in_manually_txn;
 
 	/* date and time format strings, can be set by the set_attribute method */
 	char *date_format;
@@ -93,7 +93,6 @@ typedef struct {
 
 
 typedef struct {
-
 	/* the link that owns this statement */
 	pdo_firebird_db_handle *H;
 
@@ -122,7 +121,6 @@ typedef struct {
 
 	/* the output SQLDA */
 	XSQLDA out_sqlda; /* last member */
-
 } pdo_firebird_stmt;
 
 extern const pdo_driver_t pdo_firebird_driver;
@@ -136,6 +134,8 @@ extern void php_firebird_set_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, const char
 #define php_firebird_error_with_info(d,e,el,m,ml) php_firebird_set_error(d, NULL, e, el, m, ml)
 #define php_firebird_error_stmt_with_info(s,e,el,m,ml) php_firebird_set_error(s->dbh, s, e, el, m, ml)
 
+extern bool php_firebird_commit_transaction(pdo_dbh_t *dbh, bool retain);
+
 enum {
 	PDO_FB_ATTR_DATE_FORMAT = PDO_ATTR_DRIVER_SPECIFIC,
 	PDO_FB_ATTR_TIME_FORMAT,