[Paginator] Fix type conversion during hydration of pagination limit subquery #7905
Conversation
Co-authored-by: Alexei Korolev <alexei.korolev@gmail.com>
We're keeping a BC layer in the hydrator, which prevents type conversion in scalar results. This makes bypasses such layer in order to always convert the identifier types when limiting the result set during a pagination. The main goal here is to keep the conversion DB->PHP inside of the hydrator components.
lcobucci
force-pushed
the
7890-paginator-objecti
branch
from
eb67b64
to
00ef1eb
Compare
|
@akorz can you please confirm that this solves the issue you mentioned? We'd like to release 2.6.5 today but only after your ok. |
|
@lcobucci i'll check it right now |
|
Well, bad news... I have a project with a lot of tests. Before that fix I had 101 failures in my tests. Using that fix I got 25 failures. Once I deleted this line of code https://github.com/doctrine/orm/pull/7821/files#diff-1a0c7b79092b175a78f64ed6c44c8c8cR113 my tests had passed. My previous huge diff also had the same behavior. The reason is https://github.com/doctrine/orm/pull/7821/files#diff-1a0c7b79092b175a78f64ed6c44c8c8cR113 |
|
I think you checked out wrong branch. Can you check out 2.6, not the one in referenced PR? That one was never merged. |
|
@ostrolucky it's a part my composer.lock I just took changed files from that pull requests I replaced them in my vendor folder by hands. Is it wrong way? |
|
It's wrong pull request where you took the changes from. Just do |
|
Tests are passed. Thank you guys for your great work! :) |
|
@akorz awesome! Releasing 🚀 |
Since v2.7.0 the ORM avoids using extra queries via the paginator component when maximum results isn't set on the original query. With that change, this test was not executing the code path that it was expected to run. This makes sure we trigger the forcing of custom DBAL types when hydrating the identifiers, making sure we don't introduce bugs. More info: - Forcing DBAL type conversion: doctrine#7905 - Issue on optimisation: doctrine#7829 - PR on optimisation: doctrine#7863 - Minor BC-break docs: https://github.com/doctrine/orm/blob/2.11.x/UPGRADE.md#minor-bc-break-paginator-output-walkers-arent-be-called-anymore-on-sub-queries-for-queries-without-max-results
Since v2.7.0 the ORM avoids using extra queries via the paginator component when maximum results isn't set on the original query. With that change, this test was not executing the code path that it was expected to run. This makes sure we trigger the forcing of custom DBAL types when hydrating the identifiers, making sure we don't introduce bugs. More info: - Forcing DBAL type conversion: doctrine#7905 - Issue on optimisation: doctrine#7829 - PR on optimisation: doctrine#7863 - Minor BC-break docs: https://github.com/doctrine/orm/blob/2.11.x/UPGRADE.md#minor-bc-break-paginator-output-walkers-arent-be-called-anymore-on-sub-queries-for-queries-without-max-results
Continuation of #7890 and #7903, however, using an implementation that keeps DB->PHP conversion contained in the hydrator.