Use benevolent union for scalar in queries

[VincentLanglet]

This PR could be an idea of solution for #446

I assume that before #412, the resultType of Query were less used a lot so we didn't have a lot of feedback about the type resolution. But I tried to reproduce your "bug", and even the merge of my PR

$result = $this->entityManager->createQueryBuilder()
			->select('DISTINCT IDENTITY(oi.product) AS id')
			->from(OrderItem::class, 'oi')
			->join('oi.product', 'p')
			->getQuery()
			->getResult();

were reporting list<array{id: int|numeric-string|null}>. I just give the same result with getArrayResult now.

For Identity, I found some specific solution to improve the return type,
but still, I anticipate a lot of similar issue with results like

$result = $this->entityManager->createQueryBuilder()
			->select('p.id AS id')
			->from(OrderItem::class, 'oi')
			->leftJoin('oi.product', 'p')
			->where('p.id IS NOT NULL')
			->getQuery()
			->getArrayResult();

which is reported as list<array{id: int|null}> when we would expect list<array{id: int}>. Notice that in the same way, before #412, the query was already returning list<array{id: int|null}> for getResult calls.

Since we currently cannot be sure that all those |null are true because, for example, we don't check the WHERE condition (and this would be complex). I think the best solution so far would be to use BenevolentUnion for this instead of default unions.

Before fixing the tests I'd like your opinion on such change.

[ondrejmirtes]

Hi, I need tests to understand what will this fix :) I'd say we need benevolent union even for the case without WHERE because the int|numeric-string for int keys is really annoying.

[VincentLanglet]

I'd say we need benevolent union even for the case without WHERE because the int|numeric-string for int keys is really annoying.

I added benevolent union in two situations so far:

• When there are WHERE condition in the queries
• When expression (like IDENTITY) are casted to string because we don't know how the database will behave (your case of int|numeric-string).

I updated the tests (about int|numeric-string) and added one test about where conditions @ondrejmirtes.

[VincentLanglet]

I added benevolent union in two situations so far:

• When there are WHERE condition in the queries
• When expression (like IDENTITY) are casted to string because we don't know how the database will behave (your case of int|numeric-string).

I updated the tests (about int|numeric-string) and added one test about where conditions @ondrejmirtes.

Does this solution seems ok to you @ondrejmirtes ?

This should avoid false positive from #412 but still be an improvement of query results. And possibly allow you to make new releases, unless you find others issues. :)

[ondrejmirtes]

FYI I had to revert some of your commits so I can release phpstan-doctrine with other changes. Unfortunately I currently cannot give this the work priority and the needed timeframe of multiple days in order to finish this. 84bf895...f1499e5

[VincentLanglet]

FYI I had to revert some of your commits so I can release phpstan-doctrine with other changes. Unfortunately I currently cannot give this the work priority and the needed timeframe of multiple days in order to finish this. 84bf895...f1499e5

I understand.

I recreated the PR with the commits reverted #453 then