Resort on Query::HINT_FORCE_PARTIAL_LOAD less #10798
Conversation
|
The tests didn't seem to check the other sides of the partial objects. Were those all created for the partial objects? |
|
Sorry, what do you mean by "other sides"? |
They check the properties that are part of the partial, but not the ones left out of the query, that aren't hydrated. Or at least that's what I assume considering the DQL in the comments. So there couldn't be much of a different outcome after the partial-parts were removed. |
I do not think the tests were created for testing partial objects, but for testing hydration, because "partial" is not mentioned in the title of these tests. Partial objects might have been used as a way to speed up the tests. |
A lot of our tests mention it, but I do not think it is important to the test. Maybe it was a way to have more efficient tests? Most times, removing the hint does not affect the test outcome.
greg0ire
force-pushed
the
less-partial-load
branch
from
2faa11c to
4da8d3b
Compare
|
The tests seem to be for partial loading wirh the hydrator, dont they throw an exception in 3.0 for missing properties? |
|
I will try removing support for partial objects on 3.0 and see what happens. |
|
🤔 whey I try accessing |
|
@greg0ire i was under the impression that the hydrator or UoW::createEntity would just flat out refuse to hydrate the object and bail. |
|
It does not seem to care 🙂 |
| @@ -200,7 +199,7 @@ public function testSimpleMultipleRootEntityQuery(): void | |||
| } | |||
|
|
|||
| /** | |||
| * SELECT PARTIAL u.{id, name} AS user, PARTIAL a.{id, topic} | |||
| * SELECT u AS user, a | |||
| * FROM Doctrine\Tests\Models\CMS\CmsUser u, Doctrine\Tests\Models\CMS\CmsArticle a | |||
| */ | |||
| public function testSimpleMultipleRootEntityQueryWithAliasedUserEntity(): void | |||
There was a problem hiding this comment.
I don't think these tests are about partial objects. For instance, according to the title, this one seems to be about testing that the objecthydrator works when the rsm as several root entities, and one of them is aliased. I think they still make sense, even after removing partial objects.
There was a problem hiding this comment.
Seems like the partial objects really were just unrelated to the tests.
Is this happening when you access a property that isn't part of the partial object hydration? |
|
Yes, and that's because I deliberately added that access to see what would happen. |
|
Should this target |
|
I don't think so: it only touches the test suite. Having branches as similar as possible is best when there it does not come with a stability impact, IMO. |
A lot of our tests mention it, but I do not think it is important to the test. Maybe it was a way to have more efficient tests? Most times, removing the hint does not affect the test outcome.