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

Merge 2.18.x up into 3.0.x #11202

Merged
merged 12 commits into from
Feb 1, 2024
Merged

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jan 31, 2024

greg0ire and others added 8 commits January 29, 2024 21:07
It is strictly beneficial for the Psalm baseline.
Use a more specific type for getSqlStatements()
This will help to make sure we don't lose those parts of the SQL when working on doctrine#11188.
…utput-walker-test

Cover limit/offset values in `LimitSubqueryOutputWalkerTest`
@mpdude mpdude marked this pull request as draft January 31, 2024 21:23
 Conflicts:
	phpstan-dbal2.neon
	phpstan-persistence2.neon
	phpstan.neon
	psalm-baseline.xml
	psalm.xml
	src/Persisters/Entity/BasicEntityPersister.php
	src/Query/Exec/MultiTableUpdateExecutor.php
	src/Utility/LockSqlHelper.php
	tests/Tests/ORM/Functional/Locking/LockTest.php
	tests/Tests/ORM/Tools/Pagination/LimitSubqueryOutputWalkerTest.php
@mpdude mpdude force-pushed the mergeup-2.18.x-3.0.x branch from 1696d53 to da6c6f7 Compare January 31, 2024 21:28
@mpdude mpdude marked this pull request as ready for review January 31, 2024 21:31
@mpdude
Copy link
Contributor Author

mpdude commented Jan 31, 2024

Strange, tests run on my machine both with SQLite and MySQL as the primary configured drivers

@greg0ire
Copy link
Member

Try downgrading to DBAL 3 ;)

@mpdude
Copy link
Contributor Author

mpdude commented Feb 1, 2024

So the reasons really is certain platforms generating different SQL in DBAL 3 vs 4?

How could I best write a portable test?

@greg0ire
Copy link
Member

greg0ire commented Feb 1, 2024

I think asserting SQL in the ORM is wrong. The best test for this should be a functional one. For now, I suggest you skip this test when DBAL 3 is in use, and create an issue about rewriting such tests.

@mpdude
Copy link
Contributor Author

mpdude commented Feb 1, 2024

@greg0ire I think I found a way to make the tests work for both DBAL 3 and 4.

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.

Nicely done. I think it still looks weird and we still should rewrite this test, but definitely not in this PR 👍

@greg0ire greg0ire merged commit 2e155e9 into doctrine:3.0.x Feb 1, 2024
62 checks passed
@mpdude mpdude deleted the mergeup-2.18.x-3.0.x branch February 1, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants