fix(#572): autocommit default tx immediately after DDL execute - #584
Merged
Conversation
DDL executed via fbird_prepare_ex($link,$sql,null)+fbird_execute() ran on the DEFAULT transaction but only committed at connection close (#294's free-path commit is gated on had_open_cursor, which DDL never has). Metadata locks (RDB$RELATION_FIELDS et al.) therefore lingered for the connection's lifetime, making the DDL invisible to other connections and blocking their no-wait transactions. Commit the default tx right after successful DDL execution at execute_done. Mirrors #294 guards (default tr_list head only, non-persistent links, not in MSHUTDOWN); explicit transactions keep Firebird transactional DDL semantics via the #540/#566 block. Same cursor-invalidation trade-off documented as #566. TDD: tests/ddl_default_tx_commit_001.phpt (RED without fix - 'Table unknown' on second connection while first stays open; GREEN with fix; control re-verified after stash). Extension suite 414/0. Note: this removes one lingering-lock source but does NOT by itself stop the #578 testSetIsolationLevelSerializable lock conflict (still 1/2359 on php84 full suite with this fix applied) - other lock holders remain under investigation in #578.
Closed
Review finding: the #572 DDL autocommit ignored fbt_commit()'s return value - on failure fbt_free() rolls back silently while execute returns true (silent DDL loss), and the #570 retry path below became dead code. Mirror the #570 immediate-query pattern: on failure close blocking cursors (_php_fbird_close_tx_cursors, forward-declared) and retry once; report via _php_fbird_error if the retry also fails. Drop the unused duplicate ISC_STATUS buffer. CHANGELOG: Unreleased entry for the #572 behavior change. Verification: ddl_default_tx_commit_001 + bug570_ddl_autocommit PASS, extension suite 415/0 (php85-fb3-dev).
Author
|
Hardening pushed per review Finding 1+2 ( |
The Stubs Sync Check requires --CLEAN-- sections in phpt files containing DDL. Use the house clean.inc GC (drops leftover test DBs via firebird.inc marker files).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #572. Successor of closed #581 (rebased onto post-#577 main).
Summary
DDL via
fbird_prepare_ex($link, $sql, null)+ execute ran on the default transaction but committed only at connection close (#294's free-path commit is gated onhad_open_cursor, which DDL never has). Metadata locks lingered for the connection lifetime: DDL invisible to other connections, no-wait transactions blocked (#572 customer report).Fix: commit the default tx right after successful DDL execution at
execute_donein_php_fbird_exec. Mirrors #294 guards (default tr_list head only, non-persistent, not in MSHUTDOWN). Explicit transactions keep transactional-DDL semantics via the existing #540/#566 block. Same cursor-invalidation trade-off documented as #566.TDD evidence
Table unknown T572_DDLon second connection while first stays opentests/ddl_default_tx_commit_001.phptpassesTest mirrors the driver's exact call shape (
fbird_prepare_ex(..., null)+FBIRD_CONNECT_FORCE_NEWsecond physical link).Verification
Known limitation (honest)
Removes ONE lingering-lock source but does not by itself stop the #578
testSetIsolationLevelSerializableconflict. Other lock holders under investigation in #578 (engine-levelfb_lock_printnext).