From 58f49a354313d1f4fe0030340d139684ba390321 Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 00:34:51 +0900
Subject: [PATCH 1/8] Corrected and optimized pdo_firebird transaction
 handling.

---
 ext/pdo/tests/pdo_017.phpt              |   1 -
 ext/pdo_firebird/firebird_driver.c      | 106 +++++++++++++++++-------
 ext/pdo_firebird/firebird_statement.c   |   7 +-
 ext/pdo_firebird/php_pdo_firebird_int.h |   1 +
 4 files changed, 80 insertions(+), 35 deletions(-)

diff --git a/ext/pdo/tests/pdo_017.phpt b/ext/pdo/tests/pdo_017.phpt
index bc514bff2c32b..d24aff1c15f32 100644
--- a/ext/pdo/tests/pdo_017.phpt
+++ b/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 {
diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index cbd7ee7cfd377..ebabb6d49523d 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -475,7 +475,7 @@ 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)) {
 				RECORD_ERROR(dbh);
@@ -486,6 +486,7 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 			}
 		}
 	}
+	H->in_manually_txn = 0;
 
 	if (isc_detach_database(H->isc_status, &H->db)) {
 		RECORD_ERROR(dbh);
@@ -647,8 +648,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)) {
-		RECORD_ERROR(dbh);
+	if (dbh->auto_commit && !H->in_manually_txn) {
+		if (isc_commit_retaining(H->isc_status, &H->tr)) {
+			RECORD_ERROR(dbh);
+		}
 	}
 
 free_statement:
@@ -700,8 +703,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) /* {{{ */
+/* firebird_begin_transaction */
+static bool 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;
@@ -753,15 +756,42 @@ static bool firebird_handle_begin(pdo_dbh_t *dbh) /* {{{ */
 }
 /* }}} */
 
-/* called by PDO to commit a transaction */
-static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
+/* called by PDO to start a transaction */
+static bool firebird_handle_begin(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 (dbh->auto_commit && H->tr && isc_commit_transaction(H->isc_status, &H->tr)) {
 		RECORD_ERROR(dbh);
 		return false;
 	}
+
+	if (!firebird_begin_transaction(dbh)) {
+		return false;
+	}
+
+	H->in_manually_txn = 1;
+	return true;
+}
+/* }}} */
+
+/* called by PDO to commit a transaction */
+static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+
+	if (dbh->auto_commit) {
+		if (isc_commit_retaining(H->isc_status, &H->tr)) {
+			RECORD_ERROR(dbh);
+			return false;
+		}
+	} else {
+		if (isc_commit_transaction(H->isc_status, &H->tr)) {
+			RECORD_ERROR(dbh);
+			return false;
+		}
+	}
+	H->in_manually_txn = 0;
 	return true;
 }
 /* }}} */
@@ -771,10 +801,18 @@ static bool firebird_handle_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)) {
-		RECORD_ERROR(dbh);
-		return false;
+	if (dbh->auto_commit) {
+		if (isc_rollback_retaining(H->isc_status, &H->tr)) {
+			RECORD_ERROR(dbh);
+			return false;
+		}
+	} else {
+		if (isc_rollback_transaction(H->isc_status, &H->tr)) {
+			RECORD_ERROR(dbh);
+			return false;
+		}
 	}
+	H->in_manually_txn = 0;
 	return true;
 }
 /* }}} */
@@ -792,16 +830,6 @@ static int firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sql,
 		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)) {
 		RECORD_ERROR(dbh);
@@ -844,19 +872,22 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 
 				/* ignore if the new value equals the old one */
 				if (dbh->auto_commit ^ bval) {
-					if (dbh->in_txn) {
-						if (bval) {
+					if (bval) {
+						if (H->tr && H->in_manually_txn) {
 							/* turning on auto_commit with an open transaction is illegal, because
-							   we won't know what to do with it */
+							we won't know what to do with it */
 							H->last_app_error = "Cannot enable auto-commit while a transaction is already open";
 							return false;
-						} else {
-							/* close the transaction */
-							if (!firebird_handle_commit(dbh)) {
-								break;
-							}
-							dbh->in_txn = false;
 						}
+						if (!H->tr && !firebird_begin_transaction(dbh)) {
+							return false;
+						}
+					} else {
+						/* close the transaction */
+						if (!H->tr && !firebird_handle_commit(dbh)) {
+							return false;
+						}
+						H->in_manually_txn = 0;
 					}
 					dbh->auto_commit = bval;
 				}
@@ -1011,6 +1042,14 @@ static void pdo_firebird_fetch_error_func(pdo_dbh_t *dbh, pdo_stmt_t *stmt, zval
 }
 /* }}} */
 
+/* {{{ firebird_in_transaction */
+static bool firebird_in_transaction(pdo_dbh_t *dbh)
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+	return H->tr && H->in_manually_txn;
+}
+/* }}} */
+
 static const struct pdo_dbh_methods firebird_methods = { /* {{{ */
 	firebird_handle_closer,
 	firebird_handle_preparer,
@@ -1026,7 +1065,7 @@ static const struct pdo_dbh_methods firebird_methods = { /* {{{ */
 	NULL, /* check_liveness */
 	NULL, /* get driver methods */
 	NULL, /* request shutdown */
-	NULL, /* in transaction, use PDO's internal tracking mechanism */
+	firebird_in_transaction,
 	NULL /* get gc */
 };
 /* }}} */
@@ -1107,6 +1146,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 = firebird_begin_transaction(dbh);
+	}
+
 	if (!ret) {
 		firebird_handle_closer(dbh);
 	}
diff --git a/ext/pdo_firebird/firebird_statement.c b/ext/pdo_firebird/firebird_statement.c
index 4ad51ab483d96..197ceab57a033 100644
--- a/ext/pdo_firebird/firebird_statement.c
+++ b/ext/pdo_firebird/firebird_statement.c
@@ -177,9 +177,10 @@ static int firebird_stmt_execute(pdo_stmt_t *stmt) /* {{{ */
 				;
 		}
 
-		/* commit? */
-		if (stmt->dbh->auto_commit && isc_commit_retaining(H->isc_status, &H->tr)) {
-			break;
+		if (stmt->dbh->auto_commit && !S->H->in_manually_txn) {
+			if (isc_commit_retaining(H->isc_status, &H->tr)) {
+				break;
+			}
 		}
 
 		*S->name = 0;
diff --git a/ext/pdo_firebird/php_pdo_firebird_int.h b/ext/pdo_firebird/php_pdo_firebird_int.h
index 243163b8a08ad..12206d6e0d3c5 100644
--- a/ext/pdo_firebird/php_pdo_firebird_int.h
+++ b/ext/pdo_firebird/php_pdo_firebird_int.h
@@ -69,6 +69,7 @@ typedef struct {
 
 	/* the transaction handle */
 	isc_tr_handle tr;
+	bool in_manually_txn:1;
 
 	/* the last error that didn't come from the API */
 	char const *last_app_error;

From aef1bcc0aaee9c311ace27a8518c2cac20ab9377 Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 01:17:29 +0900
Subject: [PATCH 2/8] Turn off autocommit during testing

---
 ext/pdo_firebird/tests/payload_test.phpt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ext/pdo_firebird/tests/payload_test.phpt b/ext/pdo_firebird/tests/payload_test.phpt
index 0638e47e7e13f..e093d5a831763 100644
--- a/ext/pdo_firebird/tests/payload_test.phpt
+++ b/ext/pdo_firebird/tests/payload_test.phpt
@@ -16,6 +16,9 @@ $dsn = "firebird:dbname=inet://$address/test";
 $username = 'SYSDBA';
 $password = 'masterkey';
 
-new PDO($dsn, $username, $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
+new PDO($dsn, $username, $password, [
+    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
+    PDO::ATTR_AUTOCOMMIT => false,
+]);
 ?>
 --EXPECT--

From 42b83afe01c4496d734917c98a31d3723f542e16 Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 02:20:18 +0900
Subject: [PATCH 3/8] Correct the error in the truth of the condition

---
 ext/pdo_firebird/firebird_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index ebabb6d49523d..951b21e8b3e56 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -884,7 +884,7 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 						}
 					} else {
 						/* close the transaction */
-						if (!H->tr && !firebird_handle_commit(dbh)) {
+						if (H->tr && !firebird_handle_commit(dbh)) {
 							return false;
 						}
 						H->in_manually_txn = 0;

From b5e132359f2b05e590a6217ede5be95a4f5f00ae Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 02:23:36 +0900
Subject: [PATCH 4/8] Delete the part where commit() is executed in autocommit
 mode

---
 ext/pdo_firebird/tests/bug_47415.phpt | 4 ----
 ext/pdo_firebird/tests/bug_48877.phpt | 2 --
 ext/pdo_firebird/tests/bug_53280.phpt | 2 --
 ext/pdo_firebird/tests/bug_62024.phpt | 6 ------
 ext/pdo_firebird/tests/bug_64037.phpt | 4 ----
 ext/pdo_firebird/tests/rowCount.phpt  | 4 ----
 6 files changed, 22 deletions(-)

diff --git a/ext/pdo_firebird/tests/bug_47415.phpt b/ext/pdo_firebird/tests/bug_47415.phpt
index 8349a1d50fe5d..9fb5eb660f791 100644
--- a/ext/pdo_firebird/tests/bug_47415.phpt
+++ b/ext/pdo_firebird/tests/bug_47415.phpt
@@ -15,8 +15,6 @@ $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
 $dbh->exec('CREATE TABLE test47415 (idx int NOT NULL PRIMARY KEY, txt VARCHAR(20))');
 $dbh->exec('INSERT INTO test47415 VALUES(0, \'String0\')');
 
-$dbh->commit();
-
 $query = "SELECT idx, txt FROM test47415 ORDER by idx";
 $idx = $txt = 0;
 $stmt = $dbh->prepare($query);
@@ -31,8 +29,6 @@ var_dump($stmt->rowCount());
 $stmt = $dbh->prepare('DELETE FROM test47415');
 $stmt->execute();
 
-$dbh->commit();
-
 unset($stmt);
 unset($dbh);
 ?>
diff --git a/ext/pdo_firebird/tests/bug_48877.phpt b/ext/pdo_firebird/tests/bug_48877.phpt
index e3cb6e93518d8..4415b24ad8c7f 100644
--- a/ext/pdo_firebird/tests/bug_48877.phpt
+++ b/ext/pdo_firebird/tests/bug_48877.phpt
@@ -17,7 +17,6 @@ $dbh->exec('CREATE TABLE test48877 (A integer)');
 $dbh->exec("INSERT INTO test48877 VALUES ('1')");
 $dbh->exec("INSERT INTO test48877 VALUES ('2')");
 $dbh->exec("INSERT INTO test48877 VALUES ('3')");
-$dbh->commit();
 
 $query = "SELECT * FROM test48877 WHERE A = :paramno";
 
@@ -32,7 +31,6 @@ var_dump($stmt->rowCount());
 $stmt = $dbh->prepare('DELETE FROM test48877');
 $stmt->execute();
 
-$dbh->commit();
 unset($stmt);
 unset($dbh);
 
diff --git a/ext/pdo_firebird/tests/bug_53280.phpt b/ext/pdo_firebird/tests/bug_53280.phpt
index 2a4c19a44aa8c..d8615cb03a9d4 100644
--- a/ext/pdo_firebird/tests/bug_53280.phpt
+++ b/ext/pdo_firebird/tests/bug_53280.phpt
@@ -13,7 +13,6 @@ require("testdb.inc");
 
 $dbh->exec('CREATE TABLE test53280(A VARCHAR(30), B VARCHAR(30), C VARCHAR(30))');
 $dbh->exec("INSERT INTO test53280 VALUES ('A', 'B', 'C')");
-$dbh->commit();
 
 $stmt1 = "SELECT B FROM test53280 WHERE A = ? AND B = ?";
 $stmt2 = "SELECT B, C FROM test53280 WHERE A = ? AND B = ?";
@@ -28,7 +27,6 @@ $stmth1->execute(array('A', 'B'));
 $rows = $stmth1->fetchAll(); // <------- segfault
 var_dump($rows);
 
-$dbh->commit();
 unset($stmth1);
 unset($stmth2);
 unset($stmt);
diff --git a/ext/pdo_firebird/tests/bug_62024.phpt b/ext/pdo_firebird/tests/bug_62024.phpt
index e46fad61fdad8..2c351421d26c6 100644
--- a/ext/pdo_firebird/tests/bug_62024.phpt
+++ b/ext/pdo_firebird/tests/bug_62024.phpt
@@ -14,8 +14,6 @@ require("testdb.inc");
 $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
 $dbh->exec("CREATE TABLE test62024 (ID INTEGER NOT NULL, TEXT VARCHAR(10))");
 
-$dbh->commit();
-
 //start actual test
 
 $sql = "insert into test62024 (id, text) values (?, ?)";
@@ -30,15 +28,11 @@ var_dump($res);
 $res = $sttmt->execute($args_err);
 var_dump($res);
 
-$dbh->commit();
-
 
 //teardown test data
 $sttmt = $dbh->prepare('DELETE FROM test62024');
 $sttmt->execute();
 
-$dbh->commit();
-
 unset($sttmt);
 unset($dbh);
 
diff --git a/ext/pdo_firebird/tests/bug_64037.phpt b/ext/pdo_firebird/tests/bug_64037.phpt
index a86fe5f10914e..f1034a4dacef9 100644
--- a/ext/pdo_firebird/tests/bug_64037.phpt
+++ b/ext/pdo_firebird/tests/bug_64037.phpt
@@ -17,8 +17,6 @@ $dbh->exec("INSERT INTO test64037 (ID, TEXT, COST) VALUES (1, 'test', -1.0)");
 $dbh->exec("INSERT INTO test64037 (ID, TEXT, COST) VALUES (2, 'test', -0.99)");
 $dbh->exec("INSERT INTO test64037 (ID, TEXT, COST) VALUES (3, 'test', -1.01)");
 
-$dbh->commit();
-
 $query = "SELECT * from test64037 order by ID";
 $stmt = $dbh->prepare($query);
 $stmt->execute();
@@ -31,8 +29,6 @@ var_dump($rows[2]['COST']);
 $stmt = $dbh->prepare('DELETE FROM test64037');
 $stmt->execute();
 
-$dbh->commit();
-
 unset($stmt);
 unset($dbh);
 
diff --git a/ext/pdo_firebird/tests/rowCount.phpt b/ext/pdo_firebird/tests/rowCount.phpt
index bf3a9be0b49de..5604223ec4cc3 100644
--- a/ext/pdo_firebird/tests/rowCount.phpt
+++ b/ext/pdo_firebird/tests/rowCount.phpt
@@ -15,7 +15,6 @@ $dbh->exec('CREATE TABLE test_rowcount (A VARCHAR(10))');
 $dbh->exec("INSERT INTO test_rowcount VALUES ('A')");
 $dbh->exec("INSERT INTO test_rowcount VALUES ('A')");
 $dbh->exec("INSERT INTO test_rowcount VALUES ('B')");
-$dbh->commit();
 
 $query = "SELECT * FROM test_rowcount WHERE A = ?";
 
@@ -29,14 +28,11 @@ var_dump($stmt->rowCount());
 $stmt = $dbh->prepare('UPDATE test_rowcount SET A="A" WHERE A != ?');
 $stmt->execute(array('A'));
 var_dump($stmt->rowCount());
-$dbh->commit();
 
 $stmt = $dbh->prepare('DELETE FROM test_rowcount');
 $stmt->execute();
 var_dump($stmt->rowCount());
 
-$dbh->commit();
-
 unset($stmt);
 unset($dbh);
 

From 98dc072a3f51af745ec103b1224c8bad77078872 Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 03:40:22 +0900
Subject: [PATCH 5/8] Fixed commit method when setting attributes

---
 ext/pdo_firebird/firebird_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index 951b21e8b3e56..e102415bc7da1 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -884,7 +884,7 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 						}
 					} else {
 						/* close the transaction */
-						if (H->tr && !firebird_handle_commit(dbh)) {
+						if (H->tr && isc_commit_transaction(H->isc_status, &H->tr)) {
 							return false;
 						}
 						H->in_manually_txn = 0;

From eed86209549bf4a6197a47465b5016dc834667c6 Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 11:10:39 +0900
Subject: [PATCH 6/8] refactor

---
 ext/pdo_firebird/firebird_driver.c | 86 +++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 32 deletions(-)

diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index e102415bc7da1..ee2450170a917 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -31,8 +31,9 @@
 #include "php_pdo_firebird.h"
 #include "php_pdo_firebird_int.h"
 
-static int firebird_alloc_prepare_stmt(pdo_dbh_t*, const zend_string*, XSQLDA*, isc_stmt_handle*,
-	HashTable*);
+static int firebird_alloc_prepare_stmt(pdo_dbh_t*, const zend_string*, XSQLDA*, isc_stmt_handle*, HashTable*);
+static bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain);
+static bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain);
 
 const char CHR_LETTER = 1;
 const char CHR_DIGIT = 2;
@@ -477,13 +478,9 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 
 	if (H->tr) {
 		if (dbh->auto_commit) {
-			if (isc_commit_transaction(H->isc_status, &H->tr)) {
-				RECORD_ERROR(dbh);
-			}
+			_firebird_commit_transaction(dbh, false);
 		} else {
-			if (isc_rollback_transaction(H->isc_status, &H->tr)) {
-				RECORD_ERROR(dbh);
-			}
+			_firebird_rollback_transaction(dbh, false);
 		}
 	}
 	H->in_manually_txn = 0;
@@ -649,9 +646,7 @@ 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 && !H->in_manually_txn) {
-		if (isc_commit_retaining(H->isc_status, &H->tr)) {
-			RECORD_ERROR(dbh);
-		}
+		_firebird_commit_transaction(dbh, true);
 	}
 
 free_statement:
@@ -703,8 +698,8 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un
 }
 /* }}} */
 
-/* firebird_begin_transaction */
-static bool firebird_begin_transaction(pdo_dbh_t *dbh) /* {{{ */
+/* _firebird_begin_transaction */
+static bool _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;
@@ -757,16 +752,15 @@ static bool firebird_begin_transaction(pdo_dbh_t *dbh) /* {{{ */
 /* }}} */
 
 /* called by PDO to start a transaction */
-static bool firebird_handle_begin(pdo_dbh_t *dbh) /* {{{ */
+static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (dbh->auto_commit && H->tr && isc_commit_transaction(H->isc_status, &H->tr)) {
-		RECORD_ERROR(dbh);
+	if (dbh->auto_commit && H->tr && !_firebird_commit_transaction(dbh, false)) {
 		return false;
 	}
 
-	if (!firebird_begin_transaction(dbh)) {
+	if (!_firebird_begin_transaction(dbh)) {
 		return false;
 	}
 
@@ -775,12 +769,12 @@ static bool firebird_handle_begin(pdo_dbh_t *dbh) /* {{{ */
 }
 /* }}} */
 
-/* called by PDO to commit a transaction */
-static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
+/* _firebird_commit_transaction */
+static bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (dbh->auto_commit) {
+	if (retain) {
 		if (isc_commit_retaining(H->isc_status, &H->tr)) {
 			RECORD_ERROR(dbh);
 			return false;
@@ -791,17 +785,31 @@ static bool firebird_handle_commit(pdo_dbh_t *dbh) /* {{{ */
 			return false;
 		}
 	}
+
+	return true;
+}
+/* }}} */
+
+/* called by PDO to commit a transaction */
+static bool firebird_handle_manually_commit(pdo_dbh_t *dbh) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+
+	if (!_firebird_commit_transaction(dbh, dbh->auto_commit)) {
+		return false;
+	}
+
 	H->in_manually_txn = 0;
 	return true;
 }
 /* }}} */
 
-/* called by PDO to rollback a transaction */
-static bool firebird_handle_rollback(pdo_dbh_t *dbh) /* {{{ */
+/* _firebird_rollback_transaction */
+static bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (dbh->auto_commit) {
+	if (retain) {
 		if (isc_rollback_retaining(H->isc_status, &H->tr)) {
 			RECORD_ERROR(dbh);
 			return false;
@@ -812,6 +820,20 @@ static bool firebird_handle_rollback(pdo_dbh_t *dbh) /* {{{ */
 			return false;
 		}
 	}
+
+	return true;
+}
+/* }}} */
+
+/* called by PDO to rollback a transaction */
+static bool firebird_handle_manually_rollback(pdo_dbh_t *dbh) /* {{{ */
+{
+	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+
+	if (!_firebird_rollback_transaction(dbh, dbh->auto_commit)) {
+		return false;
+	}
+
 	H->in_manually_txn = 0;
 	return true;
 }
@@ -879,12 +901,12 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 							H->last_app_error = "Cannot enable auto-commit while a transaction is already open";
 							return false;
 						}
-						if (!H->tr && !firebird_begin_transaction(dbh)) {
+						if (!H->tr && !_firebird_begin_transaction(dbh)) {
 							return false;
 						}
 					} else {
 						/* close the transaction */
-						if (H->tr && isc_commit_transaction(H->isc_status, &H->tr)) {
+						if (H->tr && !_firebird_commit_transaction(dbh, false)) {
 							return false;
 						}
 						H->in_manually_txn = 0;
@@ -1042,8 +1064,8 @@ static void pdo_firebird_fetch_error_func(pdo_dbh_t *dbh, pdo_stmt_t *stmt, zval
 }
 /* }}} */
 
-/* {{{ firebird_in_transaction */
-static bool firebird_in_transaction(pdo_dbh_t *dbh)
+/* {{{ firebird_in_manually_transaction */
+static bool firebird_in_manually_transaction(pdo_dbh_t *dbh)
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 	return H->tr && H->in_manually_txn;
@@ -1055,9 +1077,9 @@ static const struct pdo_dbh_methods firebird_methods = { /* {{{ */
 	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,
 	firebird_handle_set_attribute,
 	NULL, /* last_id not supported */
 	pdo_firebird_fetch_error_func,
@@ -1065,7 +1087,7 @@ static const struct pdo_dbh_methods firebird_methods = { /* {{{ */
 	NULL, /* check_liveness */
 	NULL, /* get driver methods */
 	NULL, /* request shutdown */
-	firebird_in_transaction,
+	firebird_in_manually_transaction,
 	NULL /* get gc */
 };
 /* }}} */
@@ -1148,7 +1170,7 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /*
 
 	H->in_manually_txn = 0;
 	if (dbh->auto_commit && !H->tr) {
-		ret = firebird_begin_transaction(dbh);
+		ret = _firebird_begin_transaction(dbh);
 	}
 
 	if (!ret) {

From 10b02c5fc8a348359855ebd94fa9e45c41761b2b Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Mon, 13 Nov 2023 12:26:52 +0900
Subject: [PATCH 7/8] add more ddl test

---
 ext/pdo_firebird/tests/ddl2.phpt | 36 ++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 ext/pdo_firebird/tests/ddl2.phpt

diff --git a/ext/pdo_firebird/tests/ddl2.phpt b/ext/pdo_firebird/tests/ddl2.phpt
new file mode 100644
index 0000000000000..5b46e60c999f5
--- /dev/null
+++ b/ext/pdo_firebird/tests/ddl2.phpt
@@ -0,0 +1,36 @@
+--TEST--
+PDO_Firebird: DDL/transactions 2
+--EXTENSIONS--
+pdo_firebird
+--SKIPIF--
+<?php require('skipif.inc'); ?>
+--ENV--
+LSAN_OPTIONS=detect_leaks=0
+--FILE--
+<?php
+require("testdb.inc");
+
+$dbh->exec("CREATE TABLE test_ddl2 (val int)");
+
+$dbh->beginTransaction();
+$dbh->exec("INSERT INTO test_ddl2 (val) VALUES (120)");
+$dbh->exec("CREATE TABLE test_ddl2_2 (val INT)");
+$dbh->rollback();
+
+$result = $dbh->query("SELECT * FROM test_ddl2");
+foreach ($result as $r) {
+    var_dump($r);
+}
+
+unset($dbh);
+echo "done\n";
+?>
+--CLEAN--
+<?php
+require("testdb.inc");
+$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
+@$dbh->exec('DROP TABLE test_ddl2');
+@$dbh->exec('DROP TABLE test_ddl2_2');
+?>
+--EXPECT--
+done

From f0ca8ff85b743d7f4ba7126c2b00dc0fb5cd8cdb Mon Sep 17 00:00:00 2001
From: SakiTakamachi <saki@sakiot.com>
Date: Tue, 14 Nov 2023 22:49:19 +0900
Subject: [PATCH 8/8] refacter

---
 ext/pdo_firebird/firebird_driver.c      | 24 +++++++++++-------------
 ext/pdo_firebird/firebird_statement.c   |  6 ++----
 ext/pdo_firebird/php_pdo_firebird_int.h |  6 ++++++
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c
index ee2450170a917..6267dfa221dd5 100644
--- a/ext/pdo_firebird/firebird_driver.c
+++ b/ext/pdo_firebird/firebird_driver.c
@@ -32,8 +32,6 @@
 #include "php_pdo_firebird_int.h"
 
 static int firebird_alloc_prepare_stmt(pdo_dbh_t*, const zend_string*, XSQLDA*, isc_stmt_handle*, HashTable*);
-static bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain);
-static bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain);
 
 const char CHR_LETTER = 1;
 const char CHR_DIGIT = 2;
@@ -478,9 +476,9 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 
 	if (H->tr) {
 		if (dbh->auto_commit) {
-			_firebird_commit_transaction(dbh, false);
+			firebird_commit_transaction(dbh, false);
 		} else {
-			_firebird_rollback_transaction(dbh, false);
+			firebird_rollback_transaction(dbh, false);
 		}
 	}
 	H->in_manually_txn = 0;
@@ -646,7 +644,7 @@ 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 && !H->in_manually_txn) {
-		_firebird_commit_transaction(dbh, true);
+		firebird_commit_transaction(dbh, true);
 	}
 
 free_statement:
@@ -756,7 +754,7 @@ static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (dbh->auto_commit && H->tr && !_firebird_commit_transaction(dbh, false)) {
+	if (dbh->auto_commit && H->tr && !firebird_commit_transaction(dbh, false)) {
 		return false;
 	}
 
@@ -769,8 +767,8 @@ static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */
 }
 /* }}} */
 
-/* _firebird_commit_transaction */
-static bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
+/* firebird_commit_transaction */
+bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
@@ -795,7 +793,7 @@ static bool firebird_handle_manually_commit(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (!_firebird_commit_transaction(dbh, dbh->auto_commit)) {
+	if (!firebird_commit_transaction(dbh, dbh->auto_commit)) {
 		return false;
 	}
 
@@ -804,8 +802,8 @@ static bool firebird_handle_manually_commit(pdo_dbh_t *dbh) /* {{{ */
 }
 /* }}} */
 
-/* _firebird_rollback_transaction */
-static bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
+/* firebird_rollback_transaction */
+bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
@@ -830,7 +828,7 @@ static bool firebird_handle_manually_rollback(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
 
-	if (!_firebird_rollback_transaction(dbh, dbh->auto_commit)) {
+	if (!firebird_rollback_transaction(dbh, dbh->auto_commit)) {
 		return false;
 	}
 
@@ -906,7 +904,7 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 						}
 					} else {
 						/* close the transaction */
-						if (H->tr && !_firebird_commit_transaction(dbh, false)) {
+						if (H->tr && !firebird_commit_transaction(dbh, false)) {
 							return false;
 						}
 						H->in_manually_txn = 0;
diff --git a/ext/pdo_firebird/firebird_statement.c b/ext/pdo_firebird/firebird_statement.c
index 197ceab57a033..f0d43bbddb29b 100644
--- a/ext/pdo_firebird/firebird_statement.c
+++ b/ext/pdo_firebird/firebird_statement.c
@@ -177,10 +177,8 @@ static int firebird_stmt_execute(pdo_stmt_t *stmt) /* {{{ */
 				;
 		}
 
-		if (stmt->dbh->auto_commit && !S->H->in_manually_txn) {
-			if (isc_commit_retaining(H->isc_status, &H->tr)) {
-				break;
-			}
+		if (stmt->dbh->auto_commit && !S->H->in_manually_txn && !firebird_commit_transaction(stmt->dbh, true)) {
+			break;
 		}
 
 		*S->name = 0;
diff --git a/ext/pdo_firebird/php_pdo_firebird_int.h b/ext/pdo_firebird/php_pdo_firebird_int.h
index 12206d6e0d3c5..62f262e6749ea 100644
--- a/ext/pdo_firebird/php_pdo_firebird_int.h
+++ b/ext/pdo_firebird/php_pdo_firebird_int.h
@@ -59,6 +59,12 @@ typedef void (*info_func_t)(char*);
 # define PDO_FIREBIRD_HANDLE_INITIALIZER NULL
 #endif
 
+extern bool _firebird_commit_transaction(pdo_dbh_t *dbh, bool retain);
+#define firebird_commit_transaction(d, r)	_firebird_commit_transaction(d, r)
+
+extern bool _firebird_rollback_transaction(pdo_dbh_t *dbh, bool retain);
+#define firebird_rollback_transaction(d, r)	_firebird_rollback_transaction(d, r)
+
 typedef struct {
 
 	/* the result of the last API call */