FIX on CanBeOneOfMany trait giving erroneous results

[Guilhem-DELAITRE]

This PR consists of 3 commits :

• 1 commit that add a test that show the dysfunction
• 1 commit that fix the trait and subsequently fix the test
• 1 commit that fix a test on raw generated sql to adapt it to new trait functionning

The origin of the problem is this :
take the relation $this->hasOne(...)->ofMany(['updated_at' => 'max','created_at' => 'max']);
for some data, this would return an entry that actually has updated_at not corresponding to the max of updated_at.

This is caused by the fact that CanBeOneOfMany generated sql doesn't bubble up subqueries aggregates.

To illustrate this, let's look at the sql generated by accessing the relation in the test I added :

• Before FIX :

• After FIX :

In the second query, we select and then join on all previous aggregates done in the subqueries, which ensures that we are really computing the current aggregate on a set of rows that match all other aggregates.

[strotgen]

After updating to Laravel 10.3, I get an error when using the has one of many feature. My relationship looks like:

return $this->hasOne(Model::class)
            ->ofMany([
                'status' => 'max',
                'updated_at' => 'max',
            ]);

And error is:

SQLSTATE[42803]: Grouping error: 7 ERROR: column "table.status" must appear in the GROUP BY clause or be used in an aggregate function (...)

any clues on why could this be happening, or should I open a new issue?

[dwightwatson]

@strotgen I've noticed the same too and opened #46394.

[Guilhem-DELAITRE]

@dwightwatson
You're totally right, your problem is caused by this PR.

I don't have time to fix RN so maybe it could be rollbacked ?

The resolution of the problem would imply adding all aggregates "bubbling up" in the group by clauses, or repeating aggregation function at each level for all aggregates in CanBeOneOfMany.
PostgreSQL is not one of those loose rules RDBMS, and actually enforce meaningfull SQL rules, that's why it breaks.

No idea how it passed the checks that operates tests with different databases, will investigate it.

Sorry for the inconvenience.

[strotgen]

It is indeed strange that the tests passed. I created a PR reverting the change. I might have some time later in the week to code a solution using your hints.

[Guilhem-DELAITRE]

It is indeed strange that the tests passed. I created a PR reverting the change. I might have some time later in the week to code a solution using your hints.

@strotgen
If you work on it, there could also be an optimization to be made where as soon as you get to an aggregate on the key, you don't need to bother with following aggregates (and the join can be made only the key of course), which means that for a relationship like :
$this->hasOne(...)->ofMany(['id' => 'max','created_at' => 'max']);, you don't need to have max(created_at) in the query because max(id) guarantee unicity.
I didn't do it because I needed to study cases of composed or even non-existing key and how the framework handles them.