fix for getting relations

[ysshir]

tables..

parent

id	name
1	parent1

child

id	parent_id	name
1	1	child1

grand_child

id	child_id	name
1	1	grand_child1
2	1	grand_child2
3	1	grand_child3

Models...

class Model_Parent extends Orm\Model {

    protected static $_table_name  = 'parent';
    protected static $_primary_key = ['id'];
    protected static $_properties  = [
        'id',
        'name',
    ];

    protected static $_has_one = [
        'child' => [
            'key_from' => 'id',
            'model_to' => 'Model_Child',
            'key_to'   => 'parent_id',
        ],
    ];
}

class Model_Child extends Orm\Model {

    protected static $_table_name  = 'child';
    protected static $_primary_key = ['id'];
    protected static $_properties  = [
        'id',
        'parent_id',
        'name',
    ];

    protected static $_belongs_to = [
        'parent' => [
            'key_from' => 'parent_id',
            'model_to' => 'Model_Parent',
            'key_to'   => 'id',
        ],
    ];

    protected static $_has_many = [
        'grand_children' => [
            'key_from' => 'id',
            'model_to' => 'Model_GrandChild',
            'key_to'   => 'child_id',
        ],
    ];
}

class Model_GrandChild extends Orm\Model {

    protected static $_table_name  = 'grand_child';
    protected static $_primary_key = ['id'];
    protected static $_properties  = [
        'id',
        'child_id',
        'name',
    ];

    protected static $_belongs_to = [
        'child' => [
            'key_from' => 'child_id',
            'model_to' => 'Model_Child',
            'key_to'   => 'id',
        ],
    ];
}

problems

// 1st
$parent = \Model_Parent::query()
                       ->related(['child', 'child.grand_children'])
                       ->get();

if (!$parent[1]->child->is_fetched('grand_children')) {
    echo 'grand_children are not loaded correctly';
}

// 2nd
$grand_children = \Model_GrandChild::query()
                                   ->related(['child', 'child.parent'])
                                   ->get();

if ($grand_children[1]->child !== $grand_children[2]->child) {
    echo 'children instances should be same?';
}

I fixed 2 above problems.

I tried to create arrays in process_row(), but I noticed 2nd problem. therefore the fix became quite big.

[ysshir]

Wow, there is another fix for another issue..

[AdamSGit]

Actually, this may be the same issue... Can you test again with last changes on 1.9 ?

[ysshir]

@AdamSGit

I’d love to but because of my poor English, I’m not sure the actual previous problem... can you give me the sample for the problem? or the test code you used?? I think we can do better communication in PHP than English. :-)

[AdamSGit]

I said that maybe your issue was fixed by the latest changes. Can you update your installation to date and test your issue again, see if it is resolved.

[ysshir]

oiic but none of my problems are resolved.

[WanWizard]

Your 2nd test is only true of you have object caching active. If not, new objects are created for every result.

[ysshir]

@WanWizard ah, sorry for my poor understanding. $grand_children[1]->child->id = 1 $grand_children[2]->child->id = 1 but the child instances are created each time. in from_array function. because from_array function doesn't care the caches..

[WanWizard]

I see the problem.

The cache check in hydration doesn't deal with duplicate objects created in the same query. Because objects are only created for new results once hydration is completed.

I'll have to think about this, as this is the most time consuming part of processing a query, it needs to remain very efficient.

[ysshir]

How to check if the singular relation is loaded or not, you use instanceof Model. but most of the time, the variable contains array, (not if it is cached objects) so it is impossible to detect.

once I added variable for checking if it singular or not for parent. but in from_array method, it is not considering the caching. so I had to change the approach.

so I changed to create instances in the process_row, not after that. it is quite big change, I think. Because you choose to use from_array method to initialize.

anyway If you have any other way to resolve my issues, I appreciate to follow.

[WanWizard]

from_array() isn't used in process_row().

The old (1.8.x) code used object creation on the fly during hydration, which made the logic long and complex, and therefore slow. This was the reason for the rewrite, which now hydrates first, and converts later using Model::forge().

The reason for doing this is that from_array() in Model::forge() works recursive, so that saves a lot of tracking logic during hydration.

I want to avoid a revert to the old complex code at all costs. That requires some thinking and testing, not something to hurriedly hack in...

It is probably the best to add a cache check in from_array() before it calls Model::forge() to create a new object from array data.

[ysshir]

from_array isn't used in process_row.

yeah, I know, the method is used in constructor, and you forge instances after process_row, right?

but the change makes instanceof Model not to work as well. I'm pretty sure why you change the process_row so I tried to less forge in process_row with closure.

hmm I should study English

[AdamSGit]

Want me to help with that Harro ?

[ysshir]

It is probably the best to add a cache check in from_array()

yeah, I agree with this.

and I also would like you to consider if we don't need the cached objects for results with from_cache(false).

[WanWizard]

As I said, it needs a proper think, because I see more issues with the current code.

For example, consider this:

$a = Model_Test::find(1);
$b = Model_Test::forge([ 'id' => 1 ]));
echo $a === $b ? "True" : "False";

what is the result? what should the result be if caching is enabled?

This might lead to Model::forge() having to return a cached object, which currently it can't as it always returns a new instance.

Also, existing model objects could be in an is_changed() state, so it's data must not be overwritten (and I think this now happens).

But the result could also contain related objects that aren't loaded with the cached object, so they should be added, and they could also be in a changed state.

[WanWizard]

Want me to help with that Harro ?

Two ( actually, one-and-a-half :smirk: ) brains know more than one....

[ysshir]

I think we need to compromise on somewhere.

and we have to use models with caution. for editing, for reference,

but what I thought was same record should be one instance in one query.

[AdamSGit]

It seem like you do have some left, tho :-)

Will think about it.

[WanWizard]

Consistency is important. If it is sometimes the same, sometimes not, we'll be in a heap of trouble...

[ysshir]

Maybe the way to load the cached object in one request or task is wrong. must be simple.

[WanWizard]

@AdamSGit This one still needs adressing, but I think that requires moving cache lookups from process_row() to from_array(), which is a major undertaking. I don't want to forge objects in process_row(), that process should be as quick as possible.

[AdamSGit]

Alright. What about the break you mentioned with relations being set as EAV ?

[WanWizard]

Is fixed too.