alancgdw's avatar

N+1 problem relationships using repeated tables

Hi, I've been trying to figure out how to properly eager load and solve a N+1 problem I'm having in a ecommerce project.

I'm using the Laravel N+1 Query Detector, and when loading the cart page, it show's this alert:

Found the following N+1 queries in this request:
Model: App\Models\Product => Relation: App\Models\ProductAttribute - You should add "with('AppModelsProductAttribute')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductAttributeOpt - You should add "with('AppModelsProductAttributeOpt')" to eager-load this relation.

My current struture is: Cart Model -> CartProduct Model -> Product Model, this having 4 relationships:

public function attribute1() {
        return $this->hasOne(ProductAttribute::class, 'id', 'product_att1_id');
    }

    public function attribute2() {
        return $this->hasOne(ProductAttribute::class, 'id', 'product_att2_id');
    }

    public function option1() {
        return $this->hasOne(ProductAttributeOpt::class, 'id', 'product_opt1_id');
    }

    public function option2() {
        return $this->hasOne(ProductAttributeOpt::class, 'id', 'product_opt2_id');
    }

And the CartProduct has this relatioship with Product:

public function product() {

        return $this->hasOne(Product::class, 'id', 'product_id')
            ->with('parent', function ($query) {
                $query->whereNotNull('id');
            })
            ->with('attribute1', function ($query) {
                $query->whereNotNull('id');
            })
            ->with('option1', function ($query) {
                $query->whereNotNull('id');
            })
            ->with('attribute2', function ($query) {
                $query->whereNotNull('id');
            })
            ->with('option2', function ($query) {
                $query->whereNotNull('id');
            })
            ;
    }

Since the attributes and options relationships are optional, I'm using whereNotNull to only eager load when those are present, but I'm still having this N+1 problem.

Removing the "withs" from this last relatioship changes the alert, asking to eager load attribute2 and option2.

Any suggestions on how to fix this?

0 likes
19 replies
cwhite's avatar
    // CartProduct
    public function product(): HasOne
    {
        return $this->hasOne(Product::class);
    }


// in code
Cart::query()
    ->with([
        'cartProducts.product' => [
            'attribute1',
            'attribute2',
            'option1',
            'option2',
        ],
    ])
    ->get();
alancgdw's avatar

@cwhite changing to this code gave me even more N+1 alerts:

Model: App\Models\Product => Relation: App\Models\ProductAttribute - You should add "with('AppModelsProductAttribute')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductAttributeOpt - You should add "with('AppModelsProductAttributeOpt')" to eager-load this relation.
Model: App\Models\Product => Relation: parent - You should add "with('parent')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductCategory - You should add "with('AppModelsProductCategory')" to eager-load this relation.
Snapey's avatar

Your whereNotNull('id') seems off

Surely you want the column name that stores the attribute1 relation id, eg

$query->whereNotNull('product_att1_id');
alancgdw's avatar

@Snapey actually doing so breaks the query, looks like the $query references the related table/model:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'product_att1_id' in 'where clause' (Connection: tenant, SQL: select * from `product_attributes` where `product_attributes`.`id` in (1) and `product_att1_id` is not null and `product_attributes`.`deleted_at` is null)
Snapey's avatar

@alancgdw ok so its checking the relation, but the ID will never be null whilst there is a row

I don't understand why you need to do this at all since if the relationship is missing then the product will just have null as the relation?

alancgdw's avatar

@Snapey explaining the situation a bit further, in the cart page I'm showing the product name (or parent name) along with it's attributes and options names:

<p>{{ $cartProduct->product->id_parent ? $cartProduct->product->parent->name : $cartProduct->product->name }} {!! $cartProduct->product->id_parent ? "<br/><small>{$cartProduct->product->attribute1->name}:{$cartProduct->product->option1->name}" . ($cartProduct->product->product_att2_id ? "<br/>{$cartProduct->product->attribute2->name}:{$cartProduct->product->option2->name}"  : '') .   '</small>'   : '' !!}</p>

But doing so without eager loading causes another N+1 alert, pointing that attribute2 and option2 should be eager loaded

alancgdw's avatar

@Snapey looking to the queries that are being used, the problem is that, since attribute1 and attribute2 are separate relationships but using the same related table, instead of eager loading both attribute in the same query, it queries each one separately:

Attributes:

select
  *
from
  `product_attributes`
where
  `product_attributes`.`id` in (1)
  and `id` is not null
  and `product_attributes`.`deleted_at` is null
select
  *
from
  `product_attributes`
where
  `product_attributes`.`id` in (2)
  and `id` is not null
  and `product_attributes`.`deleted_at` is null

Options:

select
  *
from
  `product_attribute_opts`
where
  `product_attribute_opts`.`id` in (1, 2)
  and `id` is not null
  and `product_attribute_opts`.`deleted_at` is null
select
  *
from
  `product_attribute_opts`
where
  `product_attribute_opts`.`id` in (4,6)
  and `id` is not null
  and `product_attribute_opts`.`deleted_at` is null
Snapey's avatar

@alancgdw OK, so I've thought more, and what you are trying to do in the model will never work.

What you need to do is eager load each relation, and if you want to do this automatically, then add this to the $with protected property in the Product model.

product model

protected $with=['attribute1', 'option1', 'attribute2', 'option2'];

public function attribute1()
{
	return $this->hasOne(...);
}

public function attribute2()
{
	return $this->hasOne(...);
}
//etc
public function option1()
public function option2()

test all your relations work with tinker before messing with blade code

In the view, code defensively by using null coalesce operator

{{ $cartProduct->product->option2->name ?? '' }}

This will return an empty string when the product has no option2 rather than throwing an exception

ps, only use {!! !!} if you absolutely must. Its a security vulnerability

alancgdw's avatar

@Snapey about the blade part you are right, I'll change how the relationship data is being shown.

But the N+1 problem isn't solved by eager loading automatically in this case, even setting each relationship in each model in the '$with' still gives me alerts, with even more errors:

Model: App\Models\Product => Relation: App\Models\ProductCategory - You should add "with('AppModelsProductCategory')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductAttribute - You should add "with('AppModelsProductAttribute')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductAttributeOpt - You should add "with('AppModelsProductAttributeOpt')" to eager-load this relation.
Model: App\Models\Product => Relation: App\Models\ProductAttributeOpt - You should add "with('AppModelsProductAttributeOpt')" to eager-load this relation.
alancgdw's avatar

@Snapey yes, here are my current models and relationships:

Product Model:

class Product extends Model {
    use HasFactory, SoftDeletes, LogsActivity;

    protected $with = [
        'category',
        'parent',
        'attribute1',
        'option1',
        'attribute2',
        'option2',
    ];

public function category() {
        return $this->belongsTo(ProductCategory::class, 'product_category_id');
    }
public function childs() {
        return $this->hasMany(Product::class, 'id_parent', 'id')->orderBy('ordem', 'asc')
            ->withoutTrashed();
    }

    public function parent() {
        return $this->belongsTo(Product::class, 'id_parent')->with('category');
    }

    public function attribute1() {
        return $this->hasOne(ProductAttribute::class, 'id', 'product_att1_id');
    }

    public function attribute2() {
        return $this->hasOne(ProductAttribute::class, 'id', 'product_att2_id');
    }

    public function option1() {
        return $this->hasOne(ProductAttributeOpt::class, 'id', 'product_opt1_id');
    }

    public function option2() {
        return $this->hasOne(ProductAttributeOpt::class, 'id', 'product_opt2_id');
    }
...

CartProduct Model

class CartProduct extends Model {
    use HasFactory, SoftDeletes;

    protected $with = [
        'product',
    ];

    public function product() {

        return $this->hasOne(Product::class, 'id', 'product_id');
    }
...

Cart Model:

class Cart extends Model {
    use HasFactory, SoftDeletes;

    protected $with = [
        'cartProducts'
    ];

public function cartProducts() {
        return $this->hasMany(CartProduct::class, 'cart_id', 'id');
    }
...
Snapey's avatar

@alancgdw

You can't eager load within a product definition

return $this->belongsTo(Product::class, 'id_parent')->with('category');

Use $with sparingly. It will run all the relation queries on EVERY request for the model. You might only want the product information, not all the relations, eg when editing the product.

If you only use the relationship in one place, its better to load the relations when loading the product, eg as per @cwhite example.

Snapey's avatar

@alancgdw Do you have the reverse model relationships in the ProductAttribute and AttributeOpt classes?

Snapey's avatar

@alancgdw did you test your relationships?

This

public function attribute1() {
    return $this->hasOne(ProductAttribute::class, 'id', 'product_att1_id');
}

I'm sure should be

public function attribute1() {
    return $this->hasOne(ProductAttribute::class, 'product_att1_id');
}
alancgdw's avatar

@Snapey I'm aware of that, my original code wasn't like that, as I mentioned in the other reply, I just set all the relationship of the models in their "$with" variables for testing

alancgdw's avatar

@Snapey yes, actually those relationships works normally, without N+1, in other pages and functions, like the product page view and CRUD in the Product Controller

alancgdw's avatar

@Snapey I belive the only reserve model relantioship that isn't set is the AttributeOpt belongsToMany (is this the right relationship type in this scenario?) Product, but I don't know if setting this is gonna change anything in the N+1 problem

cwhite's avatar

Why do you two relations to the same table? I think you should structure your DB more efficiently and just have a HasMany $product->attributes relation

alancgdw's avatar

@cwhite it's structured like this because a product can have variations, which in this case are being called "childs", they link to the "main product" by it's id_parent field, and each variation must consist of one or two attributes and a option for each (for example: color(att):blue(opt) or color(att):blue(opt) + size(att):big(opt)), and for that both the attribute and option ids needs to be a field of it's own in the product table so that it can be 1) optional 2) called independently.

But I'm open for suggestions on improving the current structure but keeping those rules/functionalities

Please or to participate in this conversation.