Skip to content

Don't try a transaction for the migrator on MySQL - #25038

Merged
blizzz merged 2 commits into
masterfrom
bugfix/noid/install-mysql8-with-php8
Jan 11, 2021
Merged

Don't try a transaction for the migrator on MySQL#25038
blizzz merged 2 commits into
masterfrom
bugfix/noid/install-mysql8-with-php8

Conversation

@nickvergessen

@nickvergessennickvergessen commented Jan 8, 2021

Copy link
Copy Markdown
Member

As per https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
CREATE TABLE statements automatically commit always. The only reason
this worked in the past was that PHPs PDO connection didn't check the
actual state on commit, but only checked their internal state.
But in PHP8 this was fixed:
https://github.com/php/php-src/blob/PHP-8.0/UPGRADING#L446-L450
So now commit() fails because the internal PDO connection implicitly
commited already.

As per https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
CREATE TABLE statements automatically commit always. The only reason
this worked in the past was that PHPs PDO connection didn't check the
actual state on commit, but only checked their internal state.
But in PHP8 this was fixed:
https://github.com/php/php-src/blob/PHP-8.0/UPGRADING#L446-L450
So now commit() fails because the internal PDO connection implicitly
commited already.
Signed-off-by: Joas Schilling <coding@schilljs.com>

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blizzzblizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had successful CI runs as well, against both 7.4 and 8.0

@rullzerrullzer mentioned this pull request Jan 11, 2021
14 tasks

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

@rullzer

Copy link
Copy Markdown
Member

Signed-off-by: Joas Schilling <coding@schilljs.com>
@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 11, 2021
@blizzz
blizzz merged commit 7cdc7ad into masterJan 11, 2021
@blizzz
blizzz deleted the bugfix/noid/install-mysql8-with-php8 branch January 11, 2021 17:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: install and updatetechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@nickvergessen@rullzer@ChristophWurst@blizzz