How to: complex queries with multiple models

[SpadXIII]

I have 2 Jelly models (project, item) which are related to eachother (project 1 -- * item, through HasMany and BelongsTo). How do I select the projects using a 'complex query' without doing a manual-query without any Jelly model?

The query I'm looking for (for example) is:

SELECT `projects`.*, COUNT(`items`.`id`) AS `num_items`
FROM `projects`
LEFT OUTER JOIN `items` ON (`projects`.`id` = `items`.`project_id`)
WHERE `items`.`status` <> 1 OR `items`.`status` IS NULL
GROUP BY `projects`.`id`
ORDER BY `num_items` DESC

I've tried the following, but that doesn't work:

$projects = Jelly::select('project')
    ->select('project.*', array('COUNT("items.id")', 'num_items'))
    ->join('items', 'LEFT OUTER')->on('project.id', '=', 'items.project_id')
    ->where('items.status', '<>', '1')
    ->or_where('items.status', 'IS', NULL)
    ->group_by('id')
    ->order_by('num_items')
    ->execute();

There is a fatal error because of the group_by-call. Commenting the ->group_by('id')-line gives more result, but still an error in the query:

Unknown column 'projects.num_items' in 'order clause' [ SELECT `projects`.*, COUNT(`items`.`id`) AS `num_items` FROM `projects` LEFT OUTER JOIN `items` ON (`projects`.`id` = `items`.`project_id`) WHERE `items`.`status` <> '1' OR `items`.`status` IS NULL ORDER BY `projects`.`num_items` ]

Any ideas/suggestions?

How would I go about doing this query 'manually' ? With Sprig, you could use a query-object:

$query = DB::query(Database::SELECT, $sql);
$model->load($query, FALSE);

[SpadXIII]

The group-by call fails (with a notice at line 402) at the Kohana_Database->quote_identifier() method which expects an array with 2 items but receives an array with only 1 item

[banks]

Sounds like there are two potential bugs here - one in the group_by() method and the other in the way aliasing is adding the table name to the num_items aliased column.

There is a third issue which is a feature request which is to enable use of custom queries that still return jelly collections - we'll think about how best to do this but our main aim is to make everything work through the builder.

[banks]

The second issue is cause because order_by() forces a table to be appended to the column when resolving aliases (source). I guess there is a good reason for this and probably no quick fix.

We could potentially come up with a work around something like ->order_by('_.num_items') to indicate there is no model and that it is a derived column.

[banks]

issue one I think it a bug where _table() has been used instead of _column() here and a couple of lines below.

SpadXII, can you alter this temporarily and let us know if it fixes issue 1 - it doesn't look right to me but I'm not sure it actually solves your problem.

Just to be sure - you do have the latest Database module version don't you?

[banks]

Still not suer why the group_by array would be set wrong. Can you Kohana::debug() the builder object before execute() but after all the other methods and put intin a gist or on pastie.org or something?

[SpadXIII]

edit, this is with the changes to the builder

I have the $projects (before execute) echo'd here: http://pastie.org/private/zb4giqjcjiwmiugxcelqg This is with the group_by call but without the order_by call

With both the group_by and order_by calls is here: http://pastie.org/private/deoyis0waq5by0n7ejlefw

I'll make another issue (for feature request) of the complex-queries that returns jelly-collections

[banks]

Thanks, the bug is clear - it is because in group_by() we use func_get_args() to get all args but then the array of args is passed through to parent::group_by() as an array rather than as separate arguments. I'm not sure there is a way to do this properly without using call_user_func_array() for the parent call. I'm not even sure that would work - it's certainly not ideal as it is so slow. Using _table() instead of _column() is another error in that function but not the cause of this bug.

[banks]

Looks like the only solution here would be to use:

call_user_func_array(array('parent', 'group_by'), $columns);

// Instead of

parent::group_by($columns)

Since columns is an array of values whereas group_by expects one value per argument.

@SpadXIII I can't update this to test right now - can you make this change to jelly temporarily and verify if this solves problem 1. Try changing _table() to _column() while you're there!

[banks]

Problem 2 could potentially be solved by changing this check to only prepend the current model name to the string if the field is a valid alias defined in the current model.

I think that would look something like:

    // If the field doesn't contain a model, and it's defined in our base model add that
    if (strpos($field, '.') === FALSE AND Jelly::alias($this->_model.'.'.$field))
    {
        $field = $this->_model.'.'.$field;
    }

Perhaps not the neatest but I think that logic is sound. If the field to be aliased has no model and is not defined in the current model, it is left alone. If it IS defined in the current model. The model is added and the alias will be returned correctly.

[SpadXIII]

Changed the _$this->table() to _$this->column()

Also changed the call to the parent::group_by to call_user_func_array, but that gives a (strict-)error

Non-static method Kohana_Database_Query_Builder_Select::group_by() cannot be called statically, assuming $this from compatible context Jelly_Builder

Also tried

call_user_func_array('parent::group_by', $columns);

but that doesn't work either

[banks]

Hmm calling a parent in this way seems to be a little difficult. Apparently in PHP >= 5.3 you should be able to use call_user_func_array(array($this, 'parent::group_by'), $columns);

Not sure if that works in 5.2.x though. Can you confirm?

[banks]

Hmm it looks like the better solution in this case is to completely override the parent method (assuming it's functionality is stable) and modify $this->_group_by directly in Jelly_Builder.

[SpadXIII]

This did the trick indeed:

public function group_by($columns)
{
    $columns = func_get_args();

    foreach($columns as $i => $column)
    {
        if (is_array($column))
        {
            $columns[$i][0] = $this->_column($column[0]);
        }
        else
        {
            $columns[$i] = $this->_column($column);
        }
    }

    //return parent::group_by($columns);
    return call_user_func_array(array($this, 'parent::group_by'), $columns);
}

though, the order_by column is still incorrectly coulpled to projects.num_items

ps. I'm using php 5.2.10

[banks]

Great, OK, well We'll test that in 5.3 as well and if it works in both I'm happy for now.

Can you confirm if my other change above resolves your order_by issue?

[SpadXIII]

With the above changes, the following query is generated:

SELECT `projects`.*, COUNT(`items`.`id`) AS `num_items` FROM `projects` LEFT OUTER JOIN `items` ON (`projects`.`id` = `items`.`project_id`) WHERE `items`.`status` <> '1' OR `items`.`status` IS NULL GROUP BY `projects`.`id` ORDER BY `projects`.`num_items`

This query has the num_items incorrectly coupled

[jonathangeiger]

Line 444 in Jelly_Builder should be:

return parent::order_by($this->_column($column), $direction);

Removing the TRUE lets $this->_column() know it should only join the table and column together if it comes in that way. That is:

$this->_column('model.field');
=> 'table.column'

$this->_column('field');
=> 'column'

$this->_column('model.field', FALSE);
=> 'column'

$this->_column('field', TRUE);
=> 'table.column'

[jonathangeiger]

Also, I'd rather just override group_by completely than use call_user_func_array(). Database_Query_Builder_Select's implementation is only 1 line long, after all.

[SpadXIII]

I'm not sure which line you're referring to (there's no line 444 in my Jelly_Collection_Core), but if i change the order_by method (in Jelly_Builder_Core, line 398) in a similar fashion, it works:

public function order_by($column, $direction = NULL)
{
    return parent::order_by($this->_column($column, FALSE), $direction);
}

I'm not sure, how this affects sorting for model-fields and other table-columns though. Two quick tests by adding _->orderby('id') and _->orderby('projects.id') both return an ORDER BY id in the query. This doesn't seem correct in this case: I'm expecting the field to be aliased to projects.id.

If revert the above change, the two quick tests return an ORDER BY projects.id, which seems correct to me.

[jonathangeiger]

Brain Fart. I mean Jelly_Builder.

Just take out the TRUE/FALSE altogether so you're not passing anything for the second parameter to _column() (it's optional).

[banks]

Fixes to allow correct use of group_by and non-model fields in ordering. Closed by 27db32cd4e3fd86883a24323bdfcaacbd70e322d

[banks]

Did I do this wrong Jon? I realised that I could just set join to FALSE but this could cause unintentional side effects if there is a join and an ambigous column.

Mind you I suppose that is the case anyway.

I had to just override group_by completely in the end since there is no reliable way to use call_user_func_array() between recent PHP versions anyway. In this case I think it is a safe and sensible compromise.

Should I revert my commit and just remove the join = TRUE?

[SpadXIII]

I see banks' solution did the trick. I can use _orderby('project.id'), _orderby('projects.id') and _orderby('id'). All of these get rewritten to projects.id perfectly. And besides that, the order_by('num_items') works as well (this is only surrounded in )

My query has a join, so the column 'id' has to be rewritten to projects.id in any case.

[banks]

@SpadXIII: glad it works for you. The other change introduced here that is subtle it that now doing select('*') will really mean SELECT * even if you have joins this makes the syntax much more natural. If you actually want to limit to just the values from one model (like with() does) you can use select('model.*') and it will be correctly aliased to SELECT model_table.*.

[SpadXIII]

That works indeed as you've described. In my test I tried select('*'), select('project.*') and select('projects.*') and they all are aliassed to SELECTprojects.*. With projects being the table name of the model project.

[banks]

and they all are aliassed

Oh, the point of my point is that select('*') shouldn't be aliased anymore - it should just result in SELECT * FROM ... can you confirm this it the case?

[SpadXIII]

As Jonathan said, Brain Fart .. ;)

select('*') isn't aliased indeed