Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Cache][Doctrine][DoctrineBridge][Lock][Messenger] Compatibility with ORM 3 and DBAL 4#51947
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b543f97 toec0f818Compare| 'SELECT a.* FROM (SELECT m.id AS "id", m.body AS "body", m.headers AS "headers", m.queue_name AS "queue_name", m.created_at AS "created_at", m.available_at AS "available_at", m.delivered_at AS "delivered_at" FROM messenger_messages m WHERE (m.delivered_at is null OR m.delivered_at < ?) AND (m.available_at <= ?) AND (m.queue_name = ?)) a WHERE ROWNUM <= 50', | ||
| ]; | ||
| if (!class_exists(MySQL57Platform::class)) { | ||
| // DBAL >= 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can't we detect the Oracle changes based on something related to OraclePlatform rather than checking for the removal of MySQL57Platform ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If you have an idea, please let me know. 🙂
| publicstaticfunctionprovidePlatformSql():iterable | ||
| { | ||
| yield'MySQL' => [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
DBAL 4+ separates MariaDBPlatform from MySQLPlatform. So we should add a test for it in the data provider to avoid loosing support for MariaDB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That test is already there, scroll down a little.
stof commentedOct 10, 2023
I'm wondering whether it make sense to do that work in the 5.4 branch while 6.4 is likely to be the active LTS by the time of the Doctrine major releases. |
11c0e64 tof5266abComparederrabus commentedOct 10, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
We will maintain ORM 2 for quite a while, so only making Symfony 6 compatible with ORM 3 would be fine. A big problem is though, that no Symfony 5 package (except VarExporter) is actively conflicting with ORM 3. It's a dev dependency everywhere. So basically, people will be able to install ORM 3 with Symfony 5 and flood our tracker with bug reports. 😢 |
OskarStark commentedOct 12, 2023
We could explicitly conflict with ORM 3 to avoid such situation |
derrabus commentedOct 12, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
We could've done that two years ago, yes. If we do that now, Composer will just pin an old Doctrine Bridge and move on. |
nicolas-grekas commentedOct 12, 2023
Thank you@derrabus. |
This PR was merged into the 5.4 branch.Discussion----------[DoctrineBridge] Fix DBAL 4 compatibility| Q | A| ------------- | ---| Branch? | 5.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | N/A| License | MITThis PR backports DBAL 4 compatibility changes from#51997 that were not in#51947. Sorry for messing this up.Commits-------50c0fbc Fix DBAL 4 compatibility
Uh oh!
There was an error while loading.Please reload this page.
This PR makes the 5.4 branch compatible with ORM 3 and DBAL 4.
Note: On the 5.4 branch, I cannot add ORM 3 to the root composer.json because ORM 3 requires VarExporter 6. So, only high-deps tests it is. 🤷🏻♂️