Question about ActiveQuery::findFor()

Hi guys,

I wanted to run some small code snippet by you to see if I’m missing something… let’s imagine an active record Post with a relation image defined like this:

models/Post.php

    public function getImage()
    {
        return $this->hasOne(Image::class, ['id' => 'image_id']);
    }

If the relation is called via $post->image on a record with image_id set to NULL Yii will generate a useless database query resulting in WHERE 0=1. Is there a reason for this? It could be easily be fixed by extending the ActiveQuery class and overriding the findFor method like this:

    public function findFor($name, $model)
    {
        foreach ($this->link as $attribute) {
            if ($model->$attribute) {
                return parent::findFor($name, $model);
            }
        }

        return null;
    }

But is there any downside to this?

Thank you!

Yes! You will no longer have default behavior!

Well, I guess you are right. But the result should always be the same because WHERE 0=1 returns null anyways, so there is no need to create potentially hundreds of unnecessary queries. But this might actually be an improvement on the framework level itself?

Haven’t ever experienced any significant drawback at all.

If you open PR, the core team should be able to scrutinize the code but I don’t see any significant improvement. Can you do some benchmark and document improvement?

Thanks, I’ll look into it. I haven’t done any benchmarks, but I feel that creating unnecessary queries should simply not happen if it doesn’t have to. :wink:

Hi @chocy

As you see in the reference, ActiveQuery::findFor() is called when an relation is lazily loaded.

$model = Post::findOne($id);
echo Html::img($model->image);   // image is lazily loaded by calling findFor() here

It isn’t called when a relation is eagerly loaded.

$models = Post::find()->with('image')->all(); // all images are eagerly loaded here
foreach ($models as $model) {
    echo Html::img($model->image);  // findFor() will not be called here
}

What I want to point out here is this:

  • In real world applications, there’s no good chance that ActiveQuery::findFor() will be called hundreds of times at once.
  • If your application do calls findFor() hundreds of times at once, you’d be better reconsider your design. Consider using eager-loading approach for the performance sake.
  • While the optimization of findFor() might be possible, the effect is negligible and it won’t have much impact on the performance of your application. It won’t pay the cost.
1 Like

Hi @ softark,

this is actually a very valid point, the performance boost from a few empty queries is not really measurable – thank you for your insights. I think I will end up still checking for the foreign keys and then returning on empty, just because I like a clear debugger and if I’m working with single models eagerly loading is still nice to have…

return $this->multiple ? [] : null

This was very helpful, thank you both!

1 Like