Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 11, 2021

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Jan 8, 2021

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>
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Proof is at nextcloud/mail#4319

Copy link
Member

@blizzz blizzz left a comment

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

@rullzer rullzer mentioned this pull request Jan 11, 2021
14 tasks
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

🙈

@rullzer
Copy link
Member

rullzer commented Jan 11, 2021

Signed-off-by: Joas Schilling <coding@schilljs.com>
@ChristophWurst ChristophWurst 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 master Jan 11, 2021
@blizzz blizzz deleted the bugfix/noid/install-mysql8-with-php8 branch January 11, 2021 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: install and update technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants