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

Document breaking changes about serverVersion #5797

Merged
merged 2 commits into from
Nov 20, 2022

Conversation

greg0ire
Copy link
Member

No description provided.

@bohanyang
Copy link

bohanyang commented Oct 24, 2022

On 4.0, is it necessary to use MariaDBPlatform instead of MySQL(80)Platform?
If yes, we should have a way to manually specify it to avoid unnecessary connections.

Maybe we should document and test the format like 10.9.3-MariaDB, which works against the new cleaned-up code.

Otherwise, MariaDB users have no idea how to explicitly specify it.

@greg0ire
Copy link
Member Author

On 4.0, is it necessary to use MariaDBPlatform instead of MySQL(80)Platform?

Dunno. Where did you get that idea from?

Maybe we should document and test the format like 10.9.3-MariaDB, which works against the new cleaned-up code.

I think it's a good idea. What are users typically using before? mariadb-10.9.3?

@bohanyang
Copy link

bohanyang commented Oct 24, 2022

Dunno. Where did you get that idea from?

Seems that MariaDBPlatform should be selected when using MariaDB otherwise issues like this will happen:
doctrine/DoctrineMigrationsBundle#337 (comment)

I think it's a good idea. What are users typically using before? mariadb-10.9.3?

Sure, as it's suggested by the official doc.

@greg0ire
Copy link
Member Author

Seems that MariaDBPlatform should be selected when using MariaDB otherwise issues like this will happen:
doctrine/DoctrineMigrationsBundle#337 (comment)

@bohanyang I'm not following you, please clarify what you mean and how it relates to this.

@@ -292,9 +292,6 @@ the corresponding platform class.
You can also pass the ``serverVersion`` option if you want to disable automatic
database platform detection and choose the platform version implementation explicitly.

Copy link

@bohanyang bohanyang Oct 24, 2022

Choose a reason for hiding this comment

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

What I mean is that should we change this paragraph to something like the following rather than completely removing it:

The value of `serverVersion` option should be a complete and valid version string, just like the response to a query of database version.

For example when using MySQL or MariaDB, it should be what returned by `SELECT VERSION();`,
like '10.9.3-MariaDB-1:10.9.3+maria~ubu2204'

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 if we add this to the docs, we should add it to 3.6.x IMO, or even on 3.5.x.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, let's change the docs on 3.5.x already.

@derrabus
Copy link
Member

Shouldn't a version string like 10.9.3-MariaDB be sufficient?

@bohanyang
Copy link

Shouldn't a version string like 10.9.3-MariaDB be sufficient?

Yes

@greg0ire greg0ire requested a review from derrabus October 24, 2022 09:54
@derrabus derrabus added this to the 4.0.0-beta2 milestone Nov 20, 2022
@derrabus derrabus merged commit 59c9495 into doctrine:4.0.x Nov 20, 2022
@greg0ire greg0ire deleted the document-bc branch November 20, 2022 20:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants