Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended Orm order_by() issue #134

Closed
itsa-sh opened this issue Feb 1, 2012 · 8 comments
Closed

Extended Orm order_by() issue #134

itsa-sh opened this issue Feb 1, 2012 · 8 comments
Milestone

Comments

@itsa-sh
Copy link

itsa-sh commented Feb 1, 2012

Issue

I have a bug_tracker module. Inside this module I have a Model for my bugs table (Model_Bugs). This obviously extends the Orm\Model class.

This model DOES NOT store any default ordering, but does correctly store relationships, fields, types and validation.

When I create a new instance of this model and attempt to order by a field on the the bugs table the Orm ignores the order of which i append my order_by statements. I would have thought the ORDER in which i append my order by requests (regardless of the relation/table) is the order in which the order by statement is generated; thus is not the case.

Code Example

So I then attempt to create my model instance, the related tables are used for filtering the many of possible unsolved results.

$bugs = \Admin_Bug_Tracker\Model_Bugs::find()
    ->related('project')
    ->related('author')
    ->related('owner')
    ->related('status')
    ->related('user_group')
    ->where('project_id', \Session::get('active_project_id'));

$bugs->order_by('user_group.name', 'ASC')
    ->order_by('priority', 'DESC')
    ->order_by('status.order', 'ASC');

Results (Incorrect)

ORDER BY `t0`.`priority` DESC, `t5`.`name` ASC, `t0`.`priority` DESC, `t4`.`order` ASC

So to correct this issue, I had to hard code the table name into the field...

$bugs->order_by('user_group.name', 'ASC')
    ->order_by('admin__bug_tracker__bugs.priority', 'DESC')
    ->order_by('status.order', 'ASC');

Results (Correct, though erroneous)

ORDER BY `t5`.`name` ASC, `admin__bug_tracker__bugs`.`priority` DESC, `t4`.`order` ASC

Nevertheless, this results in an SQL Error...

Fuel\Core\Database_Exception [ 1054 ]: Unknown column 'admin__bug_tracker__bugs.priority' in 'order clause' ...

And of course, the field DOES exist So I noticed I should use the reference t0 and yet again, is ignored and the results ARE NOT filtered in the order I am expecting.

ORDER BY `t0`.`priority` DESC, `t5`.`name` ASC, `t0`.`priority` DESC, `t4`.`order` ASC
@jschreuder
Copy link
Contributor

Could you show the full query instead of just the ORDER BY part, I think I know why it's happening but I'm flying blind without knowing for sure.

@itsa-sh
Copy link
Author

itsa-sh commented Feb 1, 2012

SELECT `t0`.`id` AS `t0_c0`, `t0`.`related_id` AS `t0_c1`, `t0`.`project_id` AS `t0_c2`, `t0`.`owner_id` AS `t0_c3`, `t0`.`author_id` AS `t0_c4`, `t0`.`status_id` AS `t0_c5`, `t0`.`user_group_id` AS `t0_c6`, `t0`.`name` AS `t0_c7`, `t0`.`desc` AS `t0_c8`, `t0`.`priority` AS `t0_c9`, `t0`.`created` AS `t0_c10`, `t0`.`closed` AS `t0_c11`, `t0`.`browser` AS `t0_c12`, `t1`.`id` AS `t1_c0`, `t1`.`name` AS `t1_c1`, `t1`.`link_live` AS `t1_c2`, `t1`.`link_local` AS `t1_c3`, `t1`.`desc` AS `t1_c4`, `t1`.`created` AS `t1_c5`, `t1`.`last_updated` AS `t1_c6`, `t2`.`id` AS `t2_c0`, `t2`.`group_id` AS `t2_c1`, `t2`.`username` AS `t2_c2`, `t2`.`password` AS `t2_c3`, `t2`.`email` AS `t2_c4`, `t2`.`profile_fields` AS `t2_c5`, `t2`.`group` AS `t2_c6`, `t2`.`last_login` AS `t2_c7`, `t2`.`login_hash` AS `t2_c8`, `t2`.`created_at` AS `t2_c9`, `t2`.`updated_at` AS `t2_c10`, `t3`.`id` AS `t3_c0`, `t3`.`group_id` AS `t3_c1`, `t3`.`username` AS `t3_c2`, `t3`.`password` AS `t3_c3`, `t3`.`email` AS `t3_c4`, `t3`.`profile_fields` AS `t3_c5`, `t3`.`group` AS `t3_c6`, `t3`.`last_login` AS `t3_c7`, `t3`.`login_hash` AS `t3_c8`, `t3`.`created_at` AS `t3_c9`, `t3`.`updated_at` AS `t3_c10`, `t4`.`id` AS `t4_c0`, `t4`.`name` AS `t4_c1`, `t4`.`flag` AS `t4_c2`, `t4`.`order` AS `t4_c3`, `t5`.`id` AS `t5_c0`, `t5`.`name` AS `t5_c1`, `t5`.`created_at` AS `t5_c2`
FROM `admin__bug_tracker__bugs` AS `t0`
LEFT JOIN `admin__bug_tracker__projects` AS `t1` ON (`t0`.`project_id` = `t1`.`id`)
LEFT JOIN `admin__users` AS `t2` ON (`t0`.`author_id` = `t2`.`id`)
LEFT JOIN `admin__users` AS `t3` ON (`t0`.`owner_id` = `t3`.`id`)
LEFT JOIN `admin__bug_tracker__status` AS `t4` ON (`t0`.`status_id` = `t4`.`id`)
LEFT JOIN `admin__groups` AS `t5` ON (`t0`.`user_group_id` = `t5`.`id`) 
WHERE `t0`.`project_id` = '3'
ORDER BY `t0`.`priority` DESC, `t5`.`name` ASC, `t0`.`priority` DESC, `t4`.`order` ASC

@TreeNode
Copy link

I commented out the code for the first pass-thru of the $order_by array in the query.php class (~line 618) and it worked. It didn't seem to break anything else but I'm not entirely comfortable with that solution. Hopefully this helps.

@billmn
Copy link
Contributor

billmn commented Mar 22, 2012

Any news about that?

@itsa-sh
Copy link
Author

itsa-sh commented Mar 22, 2012

I've not managed to get around to testing this out, because I've not been on the project recently. However I will try your solution shortly, and respond with the results.

@richard-myers
Copy link

Was any progress ever made on this issue? I'm looking now and it seems (as mentioned) to relate to the code around lines 605-618 and commenting out this section seems to fix the issue. The problem is I can't quite work out what this block of code is actually aiming to achieve and if its actually needed and therefore if its a valid fix or how best to fix it.

@itsa-sh
Copy link
Author

itsa-sh commented Jul 31, 2012

Currently, no solution - I couldn't even find the file as suggested above to remove a line.

It appears to be once you join additional tables into the query, that this problem occurs. I have another good example with a similar outcome.

Model_User::find()
    ->related('user_group')
    ->order_by('user_group.name', 'ASC')
    ->order_by('username', 'ASC')
    ->get();

The order_by() in this instance, overrides the previous call.

The results...

SELECT `t0`.`id` AS `t0_c0`, `t0`.`group_id` AS `t0_c1`, `t0`.`username` AS `t0_c2`, `t0`.`password` AS `t0_c3`, `t0`.`email` AS `t0_c4`, `t0`.`profile_fields` AS `t0_c5`, `t0`.`group` AS `t0_c6`, `t0`.`last_login` AS `t0_c7`, `t0`.`login_hash` AS `t0_c8`, `t0`.`created_at` AS `t0_c9`, `t0`.`updated_at` AS `t0_c10`, `t1`.`id` AS `t1_c0`, `t1`.`name` AS `t1_c1`, `t1`.`created_at` AS `t1_c2` FROM `admin__users` AS `t0` LEFT JOIN `admin__groups` AS `t1` ON (`t0`.`group_id` = `t1`.`id`) ORDER BY `t0`.`username` ASC, `t1`.`name` ASC, `t0`.`username` ASC

@WanWizard
Copy link
Member

Looked into this.

The issue here is that the query is constructed on the models' table first, which causes the order_by's on that table to be added. The order_by's on related tables are only added when the relation (the join) is added, because only then the table alias is known.

It would probably be better to use the existing order_by array, replace table names by alias while the query is build constructed, then add the order_by's at the end, in the sequence in which they are defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants