From 00dbd49445d2eb61488e0bd8ac7b30f47d4af684 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Tue, 28 Nov 2023 22:43:43 +0900 Subject: [PATCH 1/5] Added transaction isolation level and access mode --- ext/pdo_firebird/firebird_driver.c | 184 +++++++++++++----- ext/pdo_firebird/pdo_firebird.c | 5 + ext/pdo_firebird/php_pdo_firebird_int.h | 14 ++ .../tests/transaction_access_mode.phpt | 184 ++++++++++++++++++ .../transaction_isolation_level_attr.phpt | 79 ++++++++ .../transaction_isolation_level_behavior.phpt | 165 ++++++++++++++++ 6 files changed, 583 insertions(+), 48 deletions(-) create mode 100644 ext/pdo_firebird/tests/transaction_access_mode.phpt create mode 100644 ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt create mode 100644 ext/pdo_firebird/tests/transaction_isolation_level_behavior.phpt diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index 3252c2ebf6de2..4d132c0f7d2b3 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -756,51 +756,56 @@ static zend_string* firebird_handle_quoter(pdo_dbh_t *dbh, const zend_string *un /* }}} */ /* php_firebird_begin_transaction */ -static bool php_firebird_begin_transaction(pdo_dbh_t *dbh) /* {{{ */ +static bool php_firebird_begin_transaction(pdo_dbh_t *dbh, bool is_auto_commit_txn) /* {{{ */ { pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data; - char tpb[8] = { isc_tpb_version3 }, *ptpb = tpb+1; -#ifdef abies_0 - if (dbh->transaction_flags & PDO_TRANS_ISOLATION_LEVEL) { - if (dbh->transaction_flags & PDO_TRANS_READ_UNCOMMITTED) { - /* this is a poor fit, but it's all we have */ - *ptpb++ = isc_tpb_read_committed; - *ptpb++ = isc_tpb_rec_version; - dbh->transaction_flags &= ~(PDO_TRANS_ISOLATION_LEVEL^PDO_TRANS_READ_UNCOMMITTED); - } else if (dbh->transaction_flags & PDO_TRANS_READ_COMMITTED) { - *ptpb++ = isc_tpb_read_committed; - *ptpb++ = isc_tpb_no_rec_version; - dbh->transaction_flags &= ~(PDO_TRANS_ISOLATION_LEVEL^PDO_TRANS_READ_COMMITTED); - } else if (dbh->transaction_flags & PDO_TRANS_REPEATABLE_READ) { - *ptpb++ = isc_tpb_concurrency; - dbh->transaction_flags &= ~(PDO_TRANS_ISOLATION_LEVEL^PDO_TRANS_REPEATABLE_READ); - } else { - *ptpb++ = isc_tpb_consistency; - dbh->transaction_flags &= ~(PDO_TRANS_ISOLATION_LEVEL^PDO_TRANS_SERIALIZABLE); - } - } - if (dbh->transaction_flags & PDO_TRANS_ACCESS_MODE) { - if (dbh->transaction_flags & PDO_TRANS_READONLY) { - *ptpb++ = isc_tpb_read; - dbh->transaction_flags &= ~(PDO_TRANS_ACCESS_MODE^PDO_TRANS_READONLY); - } else { - *ptpb++ = isc_tpb_write; - dbh->transaction_flags &= ~(PDO_TRANS_ACCESS_MODE^PDO_TRANS_READWRITE); + /* isc_xxx are all 1 byte. */ + char tpb[5] = { isc_tpb_version3 }, *ptpb = tpb + 1; + + if (is_auto_commit_txn) { + /* + * In autocommit mode, we need to always read the latest information, that is, + * expect phantom reads, so we set read committed. + */ + *ptpb++ = isc_tpb_read_committed; + *ptpb++ = isc_tpb_rec_version; + } else { + switch (H->txn_isolation_level) { + /* + * firebird's read committed has the option to wait until other transactions + * commit or rollback if there is indeterminate data. + * Introducing too many configuration values ​​at once can cause confusion, so + * we don't support in PDO that feature yet. + * + * Also, there is information that depending on the settings, it is possible to + * reproduce behavior like read uncommited, but at least with the current firebird + * API, this is not possible. + */ + case PDO_FB_READ_COMMITTED: + *ptpb++ = isc_tpb_read_committed; + *ptpb++ = isc_tpb_rec_version; + break; + + case PDO_FB_SERIALIZABLE: + *ptpb++ = isc_tpb_consistency; + break; + + case PDO_FB_REPEATABLE_READ: + default: + *ptpb++ = isc_tpb_concurrency; + break; } } - if (dbh->transaction_flags & PDO_TRANS_CONFLICT_RESOLUTION) { - if (dbh->transaction_flags & PDO_TRANS_RETRY) { - *ptpb++ = isc_tpb_wait; - dbh->transaction_flags &= ~(PDO_TRANS_CONFLICT_RESOLUTION^PDO_TRANS_RETRY); - } else { - *ptpb++ = isc_tpb_nowait; - dbh->transaction_flags &= ~(PDO_TRANS_CONFLICT_RESOLUTION^PDO_TRANS_ABORT); - } + + if (H->is_writable_txn) { + *ptpb++ = isc_tpb_write; + } else { + *ptpb++ = isc_tpb_read; } -#endif - if (isc_start_transaction(H->isc_status, &H->tr, 1, &H->db, (unsigned short)(ptpb-tpb), tpb)) { + + if (isc_start_transaction(H->isc_status, &H->tr, 1, &H->db, (unsigned short)(ptpb - tpb), tpb)) { php_firebird_error(dbh); return false; } @@ -822,7 +827,7 @@ static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */ } } - if (!php_firebird_begin_transaction(dbh)) { + if (!php_firebird_begin_transaction(dbh, /* manually */ false)) { return false; } H->in_manually_txn = 1; @@ -871,7 +876,7 @@ static bool firebird_handle_manually_commit(pdo_dbh_t *dbh) /* {{{ */ * Reopen instead of retain because isolation level may change */ if (dbh->auto_commit) { - if (!php_firebird_begin_transaction(dbh)) { + if (!php_firebird_begin_transaction(dbh, /* auto commit mode */ true)) { return false; } } @@ -907,7 +912,7 @@ static bool firebird_handle_manually_rollback(pdo_dbh_t *dbh) /* {{{ */ * Reopen instead of retain because isolation level may change */ if (dbh->auto_commit) { - if (!php_firebird_begin_transaction(dbh)) { + if (!php_firebird_begin_transaction(dbh, /* auto commit mode */ true)) { return false; } } @@ -961,6 +966,7 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val { pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data; bool bval; + zend_long lval; switch (attr) { case PDO_ATTR_AUTOCOMMIT: @@ -979,19 +985,19 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val /* ignore if the new value equals the old one */ if (dbh->auto_commit ^ bval) { if (bval) { - /* change to auto commit mode. + /* + * 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)) { + if (!php_firebird_begin_transaction(dbh, /* auto commit mode */ true)) { return false; } } } else { - /* change to not auto commit mode. + /* + * 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)) { @@ -1052,6 +1058,69 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val zend_string_release_ex(str, 0); } return true; + + case PDO_FB_TRANSACTION_ISOLATION_LEVEL: + { + if (!pdo_get_long_param(&lval, val)) { + return false; + } + + if (H->in_manually_txn) { + pdo_raise_impl_error(dbh, NULL, "HY000", "Cannot change transaction isolation level while a transaction is already open"); + return false; + } + + /* ignore if the new value equals the old one */ + if (H->txn_isolation_level != lval) { + if (lval == PDO_FB_READ_COMMITTED || + lval == PDO_FB_REPEATABLE_READ || + lval == PDO_FB_SERIALIZABLE + ) { + /* + * Autocommit mode is always read-committed, so this setting is used the next time + * a manual transaction starts. Therefore, there is no need to immediately reopen the transaction. + */ + H->txn_isolation_level = lval; + } else { + pdo_raise_impl_error(dbh, NULL, "HY000", + "Transaction isolation level must be PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::PDO_FB_SERIALIZABLE"); + return false; + } + } + } + return true; + + case PDO_FB_WRITABLE_TRANSACTION: + { + if (!pdo_get_bool_param(&bval, val)) { + return false; + } + + if (H->in_manually_txn) { + pdo_raise_impl_error(dbh, NULL, "HY000", "Cannot change access mode while a transaction is already open"); + return false; + } + + /* ignore if the new value equals the old one */ + if (H->is_writable_txn != bval) { + H->is_writable_txn = bval; + if (dbh->auto_commit) { + if (H->tr) { + if (!php_firebird_commit_transaction(dbh, /* release */ false)) { + /* In case of error, revert the setting */ + H->is_writable_txn = !bval; + return false; + } + } + if (!php_firebird_begin_transaction(dbh, /* auto commit mode */ true)) { + /* In case of error, revert the setting */ + H->is_writable_txn = !bval; + return false; + } + } + } + } + return true; } return false; } @@ -1136,6 +1205,14 @@ static int pdo_firebird_get_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val) case PDO_FB_ATTR_TIMESTAMP_FORMAT: ZVAL_STRING(val, H->timestamp_format); return 1; + + case PDO_FB_TRANSACTION_ISOLATION_LEVEL: + ZVAL_LONG(val, H->txn_isolation_level); + return 1; + + case PDO_FB_WRITABLE_TRANSACTION: + ZVAL_BOOL(val, H->is_writable_txn); + return 1; } return 0; } @@ -1213,6 +1290,18 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* dbh->password = pestrdup(vars[5].optval, dbh->is_persistent); } + H->in_manually_txn = 0; + H->is_writable_txn = pdo_attr_lval(driver_options, PDO_FB_WRITABLE_TRANSACTION, 1); + zend_long txn_isolation_level = pdo_attr_lval(driver_options, PDO_FB_TRANSACTION_ISOLATION_LEVEL, PDO_FB_REPEATABLE_READ); + if (txn_isolation_level == PDO_FB_READ_COMMITTED || + txn_isolation_level == PDO_FB_REPEATABLE_READ || + txn_isolation_level == PDO_FB_SERIALIZABLE + ) { + H->txn_isolation_level = txn_isolation_level; + } else { + H->txn_isolation_level = PDO_FB_REPEATABLE_READ; + } + do { static char const dpb_flags[] = { isc_dpb_user_name, isc_dpb_password, isc_dpb_lc_ctype, isc_dpb_sql_role_name }; @@ -1263,9 +1352,8 @@ 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); + ret = php_firebird_begin_transaction(dbh, /* auto commit mode */ true); } if (!ret) { diff --git a/ext/pdo_firebird/pdo_firebird.c b/ext/pdo_firebird/pdo_firebird.c index efbebccb0e127..35cbfb51d9a06 100644 --- a/ext/pdo_firebird/pdo_firebird.c +++ b/ext/pdo_firebird/pdo_firebird.c @@ -57,6 +57,11 @@ PHP_MINIT_FUNCTION(pdo_firebird) /* {{{ */ REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_DATE_FORMAT", (zend_long) PDO_FB_ATTR_DATE_FORMAT); REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_TIME_FORMAT", (zend_long) PDO_FB_ATTR_TIME_FORMAT); REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_TIMESTAMP_FORMAT", (zend_long) PDO_FB_ATTR_TIMESTAMP_FORMAT); + REGISTER_PDO_CLASS_CONST_LONG("FB_TRANSACTION_ISOLATION_LEVEL", (zend_long) PDO_FB_TRANSACTION_ISOLATION_LEVEL); + REGISTER_PDO_CLASS_CONST_LONG("FB_READ_COMMITTED", (zend_long) PDO_FB_READ_COMMITTED); + REGISTER_PDO_CLASS_CONST_LONG("FB_REPEATABLE_READ", (zend_long) PDO_FB_REPEATABLE_READ); + REGISTER_PDO_CLASS_CONST_LONG("FB_SERIALIZABLE", (zend_long) PDO_FB_SERIALIZABLE); + REGISTER_PDO_CLASS_CONST_LONG("FB_WRITABLE_TRANSACTION", (zend_long) PDO_FB_WRITABLE_TRANSACTION); if (FAILURE == php_pdo_register_driver(&pdo_firebird_driver)) { return FAILURE; diff --git a/ext/pdo_firebird/php_pdo_firebird_int.h b/ext/pdo_firebird/php_pdo_firebird_int.h index af598eae98ad9..dc1e4bb7f4086 100644 --- a/ext/pdo_firebird/php_pdo_firebird_int.h +++ b/ext/pdo_firebird/php_pdo_firebird_int.h @@ -75,6 +75,8 @@ typedef struct { /* the transaction handle */ isc_tr_handle tr; bool in_manually_txn; + bool is_writable_txn; + zend_ulong txn_isolation_level; /* date and time format strings, can be set by the set_attribute method */ char *date_format; @@ -140,6 +142,18 @@ enum { PDO_FB_ATTR_DATE_FORMAT = PDO_ATTR_DRIVER_SPECIFIC, PDO_FB_ATTR_TIME_FORMAT, PDO_FB_ATTR_TIMESTAMP_FORMAT, + + /* + * transaction isolation level + * firebird does not have a level equivalent to read uncommited. + */ + PDO_FB_TRANSACTION_ISOLATION_LEVEL, + PDO_FB_READ_COMMITTED, + PDO_FB_REPEATABLE_READ, + PDO_FB_SERIALIZABLE, + + /* transaction access mode */ + PDO_FB_WRITABLE_TRANSACTION, }; #endif /* PHP_PDO_FIREBIRD_INT_H */ diff --git a/ext/pdo_firebird/tests/transaction_access_mode.phpt b/ext/pdo_firebird/tests/transaction_access_mode.phpt new file mode 100644 index 0000000000000..3f5c9bcca7013 --- /dev/null +++ b/ext/pdo_firebird/tests/transaction_access_mode.phpt @@ -0,0 +1,184 @@ +--TEST-- +PDO_Firebird: transaction access mode +--EXTENSIONS-- +pdo_firebird +--SKIPIF-- + +--XLEAK-- +A bug in firebird causes a memory leak when calling `isc_attach_database()`. +See https://github.com/FirebirdSQL/firebird/issues/7849 +--FILE-- + true, 'label' => 'writable'], + ['val' => false, 'label' => 'readonly'], +]; + +echo "========== Set attr in construct ==========\n"; + +foreach ($values as $value) { + $dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, + [ + PDO::FB_WRITABLE_TRANSACTION => $value['val'], + ], + ); + + if ($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION) === $value['val']) { + echo "OK: {$value['label']}\n"; + } else { + echo "NG: {$value['label']}\n"; + } + + unset($dbh); +} + +echo "\n"; +echo "========== Set attr in setAttribute and behavior check ==========\n"; + +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, +); + +$dbh->query("CREATE TABLE {$table} (val INT)"); + +echo "writable\n"; +var_dump($dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, true)); +if ($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION) === true) { + echo "OK: writable\n"; +} else { + echo "NG: writable\n"; +} +$dbh->query("INSERT INTO {$table} VALUES (12)"); +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "\n"; + +echo "readonly\n"; +var_dump($dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, false)); +if ($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION) === false) { + echo "OK: readonly\n"; +} else { + echo "NG: readonly\n"; +} +try { + $dbh->query("INSERT INTO {$table} VALUES (19)"); +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "\n"; +echo "========== Set attr in setAttribute while transaction ==========\n"; + +$dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, true); +$dbh->beginTransaction(); + +echo "writable to writable\n"; +try { + $dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, true); +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +var_dump($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION)); +echo "\n"; + +echo "writable to readonly\n"; +try { + $dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, false); +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +var_dump($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION)); +echo "\n"; + +$dbh->commit(); +$dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, false); +$dbh->beginTransaction(); + +echo "readonly to writable\n"; +try { + $dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, true); +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +var_dump($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION)); +echo "\n"; + +echo "readonly to readonly\n"; +try { + $dbh->setAttribute(PDO::FB_WRITABLE_TRANSACTION, false); +} catch (PDOException $e) { + echo $e->getMessage()."\n"; +} +var_dump($dbh->getAttribute(PDO::FB_WRITABLE_TRANSACTION)); + +unset($dbh); +?> +--CLEAN-- +exec('DROP TABLE transaction_access_mode'); +unset($dbh); +?> +--EXPECT-- +========== Set attr in construct ========== +OK: writable +OK: readonly + +========== Set attr in setAttribute and behavior check ========== +writable +bool(true) +OK: writable +array(1) { + [0]=> + array(2) { + ["VAL"]=> + int(12) + [0]=> + int(12) + } +} + +readonly +bool(true) +OK: readonly +SQLSTATE[42000]: Syntax error or access violation: -817 attempted update during read-only transaction +array(1) { + [0]=> + array(2) { + ["VAL"]=> + int(12) + [0]=> + int(12) + } +} + +========== Set attr in setAttribute while transaction ========== +writable to writable +SQLSTATE[HY000]: General error: Cannot change access mode while a transaction is already open +bool(true) + +writable to readonly +SQLSTATE[HY000]: General error: Cannot change access mode while a transaction is already open +bool(true) + +readonly to writable +SQLSTATE[HY000]: General error: Cannot change access mode while a transaction is already open +bool(false) + +readonly to readonly +SQLSTATE[HY000]: General error: Cannot change access mode while a transaction is already open +bool(false) diff --git a/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt b/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt new file mode 100644 index 0000000000000..09e272bf45c96 --- /dev/null +++ b/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt @@ -0,0 +1,79 @@ +--TEST-- +PDO_Firebird: transaction isolation level (Testing for setting attribute values) +--EXTENSIONS-- +pdo_firebird +--SKIPIF-- + +--XLEAK-- +A bug in firebird causes a memory leak when calling `isc_attach_database()`. +See https://github.com/FirebirdSQL/firebird/issues/7849 +--FILE-- + $level, + ], + ); + + if ($dbh->getAttribute(PDO::FB_TRANSACTION_ISOLATION_LEVEL) === $level) { + echo "OK: {$levelStr}\n"; + } else { + echo "NG: {$levelStr}\n"; + } + + unset($dbh); +} + +echo "\n"; +echo "========== Set attr in setAttribute ==========\n"; + +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, +); + +foreach ($levelStrs as $levelStr) { + $level = constant($levelStr); + + var_dump($dbh->setAttribute(PDO::FB_TRANSACTION_ISOLATION_LEVEL, $level)); + + if ($dbh->getAttribute(PDO::FB_TRANSACTION_ISOLATION_LEVEL) === $level) { + echo "OK: {$levelStr}\n"; + } else { + echo "NG: {$levelStr}\n"; + } +} + +unset($dbh); +?> +--EXPECT-- +========== Set attr in construct ========== +OK: PDO::FB_READ_COMMITTED +OK: PDO::FB_REPEATABLE_READ +OK: PDO::FB_SERIALIZABLE + +========== Set attr in setAttribute ========== +bool(true) +OK: PDO::FB_READ_COMMITTED +bool(true) +OK: PDO::FB_REPEATABLE_READ +bool(true) +OK: PDO::FB_SERIALIZABLE diff --git a/ext/pdo_firebird/tests/transaction_isolation_level_behavior.phpt b/ext/pdo_firebird/tests/transaction_isolation_level_behavior.phpt new file mode 100644 index 0000000000000..b19a00e15fb33 --- /dev/null +++ b/ext/pdo_firebird/tests/transaction_isolation_level_behavior.phpt @@ -0,0 +1,165 @@ +--TEST-- +PDO_Firebird: transaction isolation level (Testing for behavior) +--EXTENSIONS-- +pdo_firebird +--SKIPIF-- + +--XLEAK-- +A bug in firebird causes a memory leak when calling `isc_attach_database()`. +See https://github.com/FirebirdSQL/firebird/issues/7849 +--FILE-- +query("CREATE TABLE {$table} (val INT)"); + + +echo "========== default(REPEATABLE READ) ==========\n"; +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, +); +echo "begin transaction\n"; +$dbh->beginTransaction(); + +echo "insert by other transaction\n"; +$dbh_other->exec("INSERT INTO {$table} VALUES (20)"); +echo "Read\n"; +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "Close transaction and reset table\n"; +$dbh->commit(); +$dbh_other->exec("DELETE FROM {$table}"); +unset($dbh); +echo "\n"; + + +echo "========== READ COMMITTED ==========\n"; +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, + [PDO::FB_TRANSACTION_ISOLATION_LEVEL => PDO::FB_READ_COMMITTED] +); +echo "begin transaction\n"; +$dbh->beginTransaction(); + +echo "insert by other transaction\n"; +$dbh_other->exec("INSERT INTO {$table} VALUES (20)"); +echo "Read\n"; +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "Close transaction and reset table\n"; +$dbh->commit(); +$dbh_other->exec("DELETE FROM {$table}"); +unset($dbh); +echo "\n"; + + +echo "========== REPEATABLE READ ==========\n"; +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, + [PDO::FB_TRANSACTION_ISOLATION_LEVEL => PDO::FB_REPEATABLE_READ] +); +echo "begin transaction\n"; +$dbh->beginTransaction(); + +echo "insert by other transaction\n"; +$dbh_other->exec("INSERT INTO {$table} VALUES (20)"); +echo "Read\n"; +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "Close transaction and reset table\n"; +$dbh->commit(); +$dbh_other->exec("DELETE FROM {$table}"); +unset($dbh); +echo "\n"; + + +/* + * SERIALIZABLE imposes a strong lock, so the lock will not be released and + * the test will never end. There is currently no way to confirm. + * If we can set the lock timeout, it might be possible to test it, so I'll leave it as is. + */ + +/* +echo "========== SERIALIZABLE ==========\n"; +$dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, + [PDO::FB_TRANSACTION_ISOLATION_LEVEL => PDO::FB_SERIALIZABLE] +); +echo "begin transaction\n"; +$dbh->beginTransaction(); + +echo "insert by other transaction\n"; +$dbh_other->exec("INSERT INTO {$table} VALUES (20)"); +echo "Read\n"; +$r = $dbh->query("SELECT * FROM {$table}"); +var_dump($r->fetchAll()); + +echo "Close transaction and reset table\n"; +$dbh->commit(); +$dbh_other->exec("DELETE FROM {$table}"); +echo "\n"; +*/ + +unset($dbh); + +echo "done!"; +?> +--CLEAN-- +exec('DROP TABLE txn_isolation_level_behavior'); +unset($dbh); +?> +--EXPECT-- +========== default(REPEATABLE READ) ========== +begin transaction +insert by other transaction +Read +array(0) { +} +Close transaction and reset table + +========== READ COMMITTED ========== +begin transaction +insert by other transaction +Read +array(1) { + [0]=> + array(2) { + ["VAL"]=> + int(20) + [0]=> + int(20) + } +} +Close transaction and reset table + +========== REPEATABLE READ ========== +begin transaction +insert by other transaction +Read +array(0) { +} +Close transaction and reset table + +done! From 4967d0850ec248b5e6e2295de70a73127a34867c Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Fri, 1 Dec 2023 21:24:09 +0900 Subject: [PATCH 2/5] be consistent in comments --- ext/pdo_firebird/firebird_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index 4d132c0f7d2b3..72f860b71d947 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -529,7 +529,7 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */ if (H->tr) { if (dbh->auto_commit) { - php_firebird_commit_transaction(dbh, /* release */ false); + php_firebird_commit_transaction(dbh, /* retain */ false); } else { php_firebird_rollback_transaction(dbh); } @@ -822,12 +822,12 @@ static bool firebird_handle_manually_begin(pdo_dbh_t *dbh) /* {{{ */ * 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)) { + if (!php_firebird_commit_transaction(dbh, /* retain */ false)) { return false; } } - if (!php_firebird_begin_transaction(dbh, /* manually */ false)) { + if (!php_firebird_begin_transaction(dbh, /* auto commit mode */ false)) { return false; } H->in_manually_txn = 1; @@ -1000,7 +1000,7 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val * close the transaction if exists. */ if (H->tr) { - if (!php_firebird_commit_transaction(dbh, /* release */ false)) { + if (!php_firebird_commit_transaction(dbh, /* retain */ false)) { return false; } } @@ -1106,7 +1106,7 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val H->is_writable_txn = bval; if (dbh->auto_commit) { if (H->tr) { - if (!php_firebird_commit_transaction(dbh, /* release */ false)) { + if (!php_firebird_commit_transaction(dbh, /* retain */ false)) { /* In case of error, revert the setting */ H->is_writable_txn = !bval; return false; From b3caa0147eb8f33e5bbf93348083476d5740e264 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Tue, 5 Dec 2023 09:58:40 +0900 Subject: [PATCH 3/5] Made transaction isolation level settings easier to understand --- ext/pdo_firebird/firebird_driver.c | 42 ++++++++++++++---------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index 72f860b71d947..08e89f4abe688 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -761,51 +761,49 @@ static bool php_firebird_begin_transaction(pdo_dbh_t *dbh, bool is_auto_commit_t pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data; /* isc_xxx are all 1 byte. */ - char tpb[5] = { isc_tpb_version3 }, *ptpb = tpb + 1; + char tpb[4] = { isc_tpb_version3 }; + size_t tpb_size; + + /* access mode. writable or readonly */ + tpb[1] = H->is_writable_txn ? isc_tpb_write : isc_tpb_read; if (is_auto_commit_txn) { /* - * In autocommit mode, we need to always read the latest information, that is, - * expect phantom reads, so we set read committed. + * In autocommit mode, we need to always read the latest information, so we set `read committed`. */ - *ptpb++ = isc_tpb_read_committed; - *ptpb++ = isc_tpb_rec_version; + tpb[2] = isc_tpb_read_committed; + /* Ignore indeterminate data from other transactions. This option only required with `read committed`. */ + tpb[3] = isc_tpb_rec_version; + tpb_size = 4; } else { switch (H->txn_isolation_level) { /* - * firebird's read committed has the option to wait until other transactions + * firebird's `read committed` has the option to wait until other transactions * commit or rollback if there is indeterminate data. * Introducing too many configuration values ​​at once can cause confusion, so * we don't support in PDO that feature yet. - * - * Also, there is information that depending on the settings, it is possible to - * reproduce behavior like read uncommited, but at least with the current firebird - * API, this is not possible. */ case PDO_FB_READ_COMMITTED: - *ptpb++ = isc_tpb_read_committed; - *ptpb++ = isc_tpb_rec_version; + tpb[2] = isc_tpb_read_committed; + /* Ignore indeterminate data from other transactions. This option only required with `read committed`. */ + tpb[3] = isc_tpb_rec_version; + tpb_size = 4; break; case PDO_FB_SERIALIZABLE: - *ptpb++ = isc_tpb_consistency; + tpb[2] = isc_tpb_consistency; + tpb_size = 3; break; case PDO_FB_REPEATABLE_READ: default: - *ptpb++ = isc_tpb_concurrency; + tpb[2] = isc_tpb_concurrency; + tpb_size = 3; break; } } - - if (H->is_writable_txn) { - *ptpb++ = isc_tpb_write; - } else { - *ptpb++ = isc_tpb_read; - } - - if (isc_start_transaction(H->isc_status, &H->tr, 1, &H->db, (unsigned short)(ptpb - tpb), tpb)) { + if (isc_start_transaction(H->isc_status, &H->tr, 1, &H->db, tpb_size, tpb)) { php_firebird_error(dbh); return false; } From 04235aa9cbfc14fefbe8a6f0a23034fa8897ecf7 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Tue, 5 Dec 2023 10:59:28 +0900 Subject: [PATCH 4/5] change to raise a value error if an invalid value is passed to PDO::FB_TRANSACTION_ISOLATION_LEVEL. --- ext/pdo_firebird/firebird_driver.c | 8 +++--- .../transaction_isolation_level_attr.phpt | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index 08e89f4abe688..a4c1a67601dbb 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -1080,8 +1080,8 @@ static bool pdo_firebird_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val */ H->txn_isolation_level = lval; } else { - pdo_raise_impl_error(dbh, NULL, "HY000", - "Transaction isolation level must be PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::PDO_FB_SERIALIZABLE"); + zend_value_error("PDO::FB_TRANSACTION_ISOLATION_LEVEL must be a valid transaction isolation level " + "(PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::FB_SERIALIZABLE)"); return false; } } @@ -1297,7 +1297,9 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* ) { H->txn_isolation_level = txn_isolation_level; } else { - H->txn_isolation_level = PDO_FB_REPEATABLE_READ; + zend_value_error("PDO::FB_TRANSACTION_ISOLATION_LEVEL must be a valid transaction isolation level " + "(PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::FB_SERIALIZABLE)"); + ret = 0; } do { diff --git a/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt b/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt index 09e272bf45c96..3f3ccb402ec96 100644 --- a/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt +++ b/ext/pdo_firebird/tests/transaction_isolation_level_attr.phpt @@ -41,6 +41,22 @@ foreach ($levelStrs as $levelStr) { unset($dbh); } +echo "Invalid value\n"; +try { + $dbh = new PDO( + PDO_FIREBIRD_TEST_DSN, + PDO_FIREBIRD_TEST_USER, + PDO_FIREBIRD_TEST_PASS, + [ + PDO::FB_TRANSACTION_ISOLATION_LEVEL => PDO::ATTR_AUTOCOMMIT, // Invalid value + ], + ); +} catch (Throwable $e) { + echo $e->getMessage()."\n"; +} + +unset($dbh); + echo "\n"; echo "========== Set attr in setAttribute ==========\n"; @@ -62,6 +78,13 @@ foreach ($levelStrs as $levelStr) { } } +echo "Invalid value\n"; +try { + $dbh->setAttribute(PDO::FB_TRANSACTION_ISOLATION_LEVEL, PDO::ATTR_AUTOCOMMIT); // Invalid value +} catch (Throwable $e) { + echo $e->getMessage()."\n"; +} + unset($dbh); ?> --EXPECT-- @@ -69,6 +92,8 @@ unset($dbh); OK: PDO::FB_READ_COMMITTED OK: PDO::FB_REPEATABLE_READ OK: PDO::FB_SERIALIZABLE +Invalid value +PDO::FB_TRANSACTION_ISOLATION_LEVEL must be a valid transaction isolation level (PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::FB_SERIALIZABLE) ========== Set attr in setAttribute ========== bool(true) @@ -77,3 +102,5 @@ bool(true) OK: PDO::FB_REPEATABLE_READ bool(true) OK: PDO::FB_SERIALIZABLE +Invalid value +PDO::FB_TRANSACTION_ISOLATION_LEVEL must be a valid transaction isolation level (PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, or PDO::FB_SERIALIZABLE) From af3de5e5406ab07e35a3254f0097b47262a03671 Mon Sep 17 00:00:00 2001 From: SakiTakamachi Date: Tue, 5 Dec 2023 15:20:39 +0900 Subject: [PATCH 5/5] [ci skip] NEWS / UPGRADING --- NEWS | 2 ++ UPGRADING | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/NEWS b/NEWS index 68128f2128378..ef414e77329b9 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,8 @@ PDO_DBLIB: PDO_FIREBIRD: . Fixed setAttribute and getAttribute (SakiTakamachi) + . Feature: Add transaction isolation level and mode settings to pdo_firebird + (SakiTakamachi) PDO_MYSQL: . Fixed setAttribute and getAttribute (SakiTakamachi) diff --git a/UPGRADING b/UPGRADING index 2334b1abbf4e8..3db4b3757f9bf 100644 --- a/UPGRADING +++ b/UPGRADING @@ -200,6 +200,10 @@ PHP 8.4 UPGRADE NOTES - PDO_FIREBIRD: . getAttribute, enabled to get values ​​of FB_ATTR_DATE_FORMAT, FB_ATTR_TIME_FORMAT, FB_ATTR_TIMESTAMP_FORMAT. + . Added new attributes to specify transaction isolation level and access mode. + Along with these, five constants (PDO::FB_TRANSACTION_ISOLATION_LEVEL, + PDO::FB_READ_COMMITTED, PDO::FB_REPEATABLE_READ, PDO::FB_SERIALIZABLE, + PDO::FB_WRITABLE_TRANSACTION) have been added. - PDO_MYSQL: . getAttribute, enabled to get the value of ATTR_FETCH_TABLE_NAMES.