2

I've got 4 tables:

structure

My relationships should work like this:

Items can only have one size, color and category.

This should be working but it's really not. The query generated returns wrong results.

Here are my model files:

<?php

class Shop_Item extends Eloquent
{
    public static $table = 'items';
    public static $timestamps = false;

    public function scategory() {
        return $this->has_one('Shop_Category','id');
    }

    public function ssize() {
        return $this->has_one('Shop_Size','id');
    }

    public function scolor() {
        return $this->has_one('Shop_Color','id');
    }
}

The rest of the model files for the remaining tables are the same (except table name and model name).

<?php
class Shop_Category extends Eloquent
{
    public static $table = 'item_categories';
    public static $timestamps = false;
}

So when I try to access the extra values (size->name, color->name, category->name), I get wrong results.

I have two test records in my database: Item 1 and Item 2 with different color, size and category. Item 1 is blue and have the size of M, Item 2 is green and have the size of XL, but not in the returned query. Which shows me, that Item 2 is red and have the size of S.

Controller:

<?php

class Admin_Shop_Controller extends Base_Controller {

    public function action_index() {

        $items = Shop_item::order_by('name')->paginate(10,array('id','name','price','sex','visible','color','size','category'));
        return View::make('admin.shop.index')->with('items', $items);
    }

View:

@forelse($items->results as $i)
{{ $i->name }}
{{ $i->price }}
{{ $i->sex }}
{{ $i->scategory->name }}
{{ $i->scolor->name }}
{{ $i->ssize->name }}

<a href = "{{ URL::to('admin/shop/edit/'.$i->id) }}">Edit</a>
<a href = "#">Delete</a>

@empty
    There are no items in the shop.
@endforelse

Queries generated:

0.23ms  
SELECT COUNT(`id`) AS `aggregate` FROM `items`
0.28ms  
SELECT `id`, `name`, `price`, `sex`, `visible`, `color`, `size`, `category` FROM `items` ORDER BY `name` ASC LIMIT 10 OFFSET 0
0.25ms  
SELECT * FROM `item_categories` WHERE `id` IN ('1', '2')
0.21ms  
SELECT * FROM `item_sizes` WHERE `id` IN ('1', '2')
0.36ms  
SELECT * FROM `item_colors` WHERE `id` IN ('1', '2')

Note that in the view if I access these values from the other table like this:

{{ Shop_Color::find($i->color)->name }}

It gets me the right result, but I really don't want to query the database n+3 times because of this. Any suggestions what am I doing wrong?

Edit: Still no luck. :( I've done the changes you listed, experimented with them but this thing still not working. Current error is :

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'id' in 'where clause'
SQL: SELECT * FROM `item_colors` WHERE `id` IN (?)

Bindings: array (
  0 => 0,
)

I don't know why it looks for an id, I've changed all references to the child tables and renamed columns appropriately. :(

peaks
  • 167
  • 2
  • 11

2 Answers2

1

Number of queries !== Performance. In your case, you're doing nearly-atomic queries (the where id in are all done vs. primary indices, which will make them fast - as fast as a JOIN if you exclude the returning-result process time). If you nevertheless want to solve the n+1 query problem, Eloquent has a solution for this: eager-loading.

Instead of this:

Shop_item::order_by('name')

Use this:

Shop_item::with(array("scategory", "ssize", "scolor"))->order_by('name')

You should see only one query by using this. The doc for this feature is at: http://laravel.com/docs/database/eloquent#eager

Sébastien Renauld
  • 19,203
  • 2
  • 46
  • 66
1

There are quite a few things going on here.

  1. I would name your reference columns thing_id... so that's color_id, size_id and category_id. This will allow you to set up relationships named 'color', 'size' and 'category', instead of 'sthing'.

  2. You need belongs_to() instead of has_one(). Laravel assumes that the ID will be like thing_id, so if you've updated as 1 above then you can update your references like $this->belongs_to('Shop_Size'). If not, then you should use the reference column here, like $this->belongs_to('Shop_Size', 'size').

  3. When you use Eloquent models it's best not to restrict the columns - you might have logic in your model that depends on them all being there.

  4. You can use eager loading to improve the queries, but the way Eloquent works it will still need a query per relationship. Have a look at this line for action_index()

    $items = Shop_Item::with(array('color', 'size', 'category'))
                  ->order_by('name')->paginate(10);

After all of the edits above you will be able to write code like this in your view...

@forelse ($items->results as $item)

    <p>Color: {{ $item->color->name }}</p>

@else
    <p class="no-results">There are no items in the shop.</p>
@endforelse

Most of this is covered in the Eloquent docs, particularly Relationships and Eager Loading.

Phill Sparks
  • 20,000
  • 3
  • 33
  • 46
  • This would change nothing in the sense, that I'd still get the wrong results. As I said, I get the wrong results back, because my second test item, gets the wrong size and wrong color. (So not accessing these values and paginating the whole isn't the problem.) – peaks Apr 11 '13 at 13:43
  • Have you followed all of my steps? Number 2 is quite critical because this is why all of your relationships are getting the wrong IDs in their queries. If you change nothing else you should `$this->has_one('Shop_Size', 'id')` to `$this->has_one('Shop_Size', 'size')` (for each relationship). – Phill Sparks Apr 11 '13 at 13:45
  • Which would end me up with this: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'color' in 'where clause' SQL: SELECT * FROM `item_colors` WHERE `color` IN (?, ?) Bindings: array ( 0 => 1, 1 => 2, ) Which shows me that the field I assign there is looking for the wrong column. – peaks Apr 11 '13 at 13:48
  • Ah, of course... you need to use `belongs_to()` not `has_one()` (they're a little backwards in Laravel). – Phill Sparks Apr 11 '13 at 13:49