Repository navigation
[WIP] Fix: Corrected and optimized pdo_firebird transaction handling. - #12657
Closed
SakiTakamachi wants to merge 8 commits into
Closed
SakiTakamachi wants to merge 8 commits into
SakiTakamachi wants to merge 8 commits into
Conversation
Member
Author
|
Some tests failed. It seems there is still room for improvement. done |
SakiTakamachi
marked this pull request as draft
November 12, 2023 15:59
Member
Author
|
Hmm. The timeout may be caused by starting a transaction in the constructor. It seems likely that I will either have to allow a timeout or change the implementation. I am implementing this to reduce the overhead when starting a transaction, but I think I need to think about it a bit. Maybe it's because the mock server doesn't assume transactional operations. For this test, I try turning off autocommit. done |
SakiTakamachi
marked this pull request as ready for review
November 13, 2023 02:59
Member
Author
|
@Girgias |
SakiTakamachi
force-pushed
the
fix/pdo_firebird_transation
branch
from
November 13, 2023 03:27
e7e52c3 to
10b02c5
Compare
Member
Author
|
|
SakiTakamachi
force-pushed
the
fix/pdo_firebird_transation
branch
from
November 14, 2023 13:49
5df0e41 to
f0ca8ff
Compare
Member
Author
|
There doesn't seem to be a fix anymore. |
SakiTakamachi
marked this pull request as draft
November 17, 2023 02:22
SakiTakamachi
changed the title
pdo_firebird transaction handling.pdo_firebird transaction handling.
Member
Author
|
It seems better to merge the error handling changes first, so I'll make this a draft. |
Girgias
pushed a commit
that referenced
this pull request
Nov 27, 2023
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.
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
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.
Follow up #12545
Firebird doesn't have anything like an autocommit mode, so we need to implement such behavior on the pdo side. Also, whatever we do with firebird, we must operate within a transaction.
This means that in many cases a transaction must already be started in order to achieve autocommit mode.
To begin with, I'm a little doubtful that there's a need to emulate autocommit mode with pdo since it doesn't exist in firebird. I'm starting to feel like it's okay to decide that autocommit mode is not supported.I was able to implement it