fuel / orm

Fuel PHP Framework - Fuel v1.x ORM
http://fuelphp.com/docs/packages/orm/intro.html
151 stars 95 forks source link

Second-degree relations not being eager-loaded #450

Closed romosan closed 1 year ago

romosan commented 1 year ago

At least since the commit 8483d217f402a39ecc06ebd8d4a771194edd9f29 (rewritten object hydration), second-degree relations don't seem to work propely. I am referring to something like ['related' => 'first.second'] where, in my case, first is a has_many relation and second a belongs_to relation.

This could be the same problem as #439 but I have opened a new issue because the old one is closed and the code has since changed. Judging by the current code, the second-degree relation gets added to the first-degree relations list, instead of a particular record. In \Orm\Query::process_row:

                // store it, but do not overwrite an existing result!
                if ($model['singular'] === true)
                {
                    if ( ! array_key_exists($current, $pointers[$parent]))
                    {
                        $pointers[$parent][$current] = $record;
                    }

Should be perhaps something like:

                // store it, but do not overwrite an existing result!
                if ($model['singular'] === true)
                {
                    if ( ! array_key_exists($current, $pointers[$parent][$parent_pk]))
                    {
                        $pointers[$parent][$parent_pk][$current] = $record;
                    }

But please note that $parent_pk is not defined and I am not sure how it should be and whether that would be enough.

WanWizard commented 1 year ago

I assume you have tested this using the 1.9-dev code?

romosan commented 1 year ago

Yes, latest version and various versions in-between.

WanWizard commented 1 year ago

Ok, I'll do some debugging, haven't bumped into this issue myself.

WanWizard commented 1 year ago

I've tested relations of different types, to 4 deep (so "A.B.C" on the root object).

Let me know if this addresses your issue too.

romosan commented 1 year ago

Yes, it does!

WanWizard commented 1 year ago

Thanks for the feedback.