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

MySQL 8.4 Platform #6385

Merged
merged 1 commit into from
May 16, 2024
Merged

MySQL 8.4 Platform #6385

merged 1 commit into from
May 16, 2024

Conversation

wmouwen
Copy link
Contributor

@wmouwen wmouwen commented May 5, 2024

Q A
Type improvement
Fixed issues none

Summary

MySQL 8.4 introduces a new set of reserved keywords (link). This PR adds a new platform definition for MySQL 8.4 and higher.

Related

@derrabus
Copy link
Member

derrabus commented May 5, 2024

Can you please rebase your PR, so #6386 is taken into account and we test your PR against MySQL 8.4?

@wmouwen
Copy link
Contributor Author

wmouwen commented May 5, 2024

The current test that fails is due to a reported MySQL bug: https://bugs.mysql.com/bug.php?id=114876

Details

There was 1 failure:

1) Doctrine\DBAL\Tests\Functional\ExceptionTest::testInvalidUserName
Failed asserting that exception of type "Doctrine\DBAL\Exception\DriverException" matches expected exception "Doctrine\DBAL\Exception\ConnectionException". Message was: "An exception occurred in the driver: Plugin 'mysql_native_password' is not loaded" at
/home/runner/work/dbal/dbal/src/Driver/Mysqli/Exception/ConnectionFailed.php:34
/home/runner/work/dbal/dbal/src/Driver/Mysqli/Driver.php:54
/home/runner/work/dbal/dbal/src/Connection.php:378
/home/runner/work/dbal/dbal/tests/Functional/ExceptionTest.php:340
/home/runner/work/dbal/dbal/tests/Functional/ExceptionTest.php:303
.

FAILURES!
Tests: 4480, Assertions: 6696, Failures: 1, Skipped: 722, Incomplete: 5.

@greg0ire
Copy link
Member

greg0ire commented May 5, 2024

That was addressed in #6388, wasn't it?

@wmouwen
Copy link
Contributor Author

wmouwen commented May 7, 2024

That was addressed in #6388, wasn't it?

Yep, sure is. Seems like IBM fixed their download issues as well. Could someone re-run the checks?

@derrabus derrabus closed this May 7, 2024
@derrabus derrabus reopened this May 7, 2024
greg0ire
greg0ire previously approved these changes May 14, 2024
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

The code looks OK to me. @derrabus do we have a statement somewhere explaining why it's OK to target 3.9.x here?

@derrabus
Copy link
Member

do we have a statement somewhere explaining why it's OK to target 3.9.x here?

Support for newer database platforms is probably okay-ish…? 🫣

@greg0ire
Copy link
Member

I think so as well, although it cannot quite formulate why we have that exception.

@wmouwen
Copy link
Contributor Author

wmouwen commented May 15, 2024

The docs make it look like MySQL 8.4 is explicitly supported by DBAL 3.*, however these new keywords not getting quoted can be seen as a bug. So instead of new platform support, you could call it a bugfix?

@greg0ire
Copy link
Member

Indeed. Plus, Doctrine does not crash with PlatformNotSupportedYet or similar upon using 8.4 right now, so it could definitely be seen as a fix.

@derrabus
Copy link
Member

MySQL 8.4 also removed a couple of keywords, see https://dev.mysql.com/doc/refman/8.4/en/keywords.html#keywords-removed-in-current-series. How shall we proceed with those?

@wmouwen
Copy link
Contributor Author

wmouwen commented May 16, 2024

MySQL 8.4 also removed a couple of keywords, see https://dev.mysql.com/doc/refman/8.4/en/keywords.html#keywords-removed-in-current-series. How shall we proceed with those?

Those can be removed using array_diff() in the MySQL84Keywords::getKeywords() method. Could do so in this PR, could do so in a new PR as well.

@derrabus
Copy link
Member

Let's do that in a follow-up. Thanks!

@derrabus derrabus merged commit 8141266 into doctrine:3.9.x May 16, 2024
92 checks passed
@wmouwen wmouwen deleted the mysql-8.4-keywords branch May 16, 2024 09:05
derrabus added a commit to derrabus/dbal that referenced this pull request May 16, 2024
* 3.9.x:
  MySQL 8.4 Platform (doctrine#6385)
derrabus added a commit to derrabus/dbal that referenced this pull request May 16, 2024
* 3.9.x:
  MySQL 8.4 Platform (doctrine#6385)
derrabus added a commit to derrabus/dbal that referenced this pull request May 16, 2024
* 4.1.x:
  MySQL 8.4 Platform (doctrine#6385)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants