Bug 629147 - ordering recipes by status or result raises an exception
Summary: ordering recipes by status or result raises an exception
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: web UI
Version: 0.5
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Raymond Mancy
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 632609
TreeView+ depends on / blocked
 
Reported: 2010-09-01 04:38 UTC by Dan Callaghan
Modified: 2019-05-22 13:34 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-09-17 02:20:00 UTC
Embargoed:


Attachments (Terms of Use)

Description Dan Callaghan 2010-09-01 04:38:23 UTC
Sorting the recipes list by status or result gives a 500.

https://beaker.engineering.redhat.com/recipes/?list_tgp_no=1&list_tgp_limit=50&list_tgp_ordering=-id&list_tgp_order=status.status

https://beaker.engineering.redhat.com/recipes/?list_tgp_no=1&list_tgp_limit=50&list_tgp_ordering=-id&list_tgp_order=result.result

In both cases it looks like the generated SQL is missing a join:

OperationalError: (OperationalError) (1054, "Unknown column 'task_status.status' in 'order clause'") u'SELECT recipe.id AS recipe_id, machine_recipe.id AS machine_recipe_id, recipe.recipe_set_id AS recipe_recipe_set_id, recipe.distro_id AS recipe_distro_id, recipe.system_id AS recipe_system_id, recipe.result_id AS recipe_result_id, recipe.status_id AS recipe_status_id, recipe.start_time AS recipe_start_time, recipe.finish_time AS recipe_finish_time, recipe._host_requires AS recipe__host_requires, recipe._distro_requires AS recipe__distro_requires, recipe.kickstart AS recipe_kickstart, recipe.type AS recipe_type, recipe.ttasks AS recipe_ttasks, recipe.ptasks AS recipe_ptasks, recipe.wtasks AS recipe_wtasks, recipe.ftasks AS recipe_ftasks, recipe.ktasks AS recipe_ktasks, recipe.whiteboard AS recipe_whiteboard, recipe.ks_meta AS recipe_ks_meta, recipe.kernel_options AS recipe_kernel_options, recipe.kernel_options_post AS recipe_kernel_options_post, recipe.role AS recipe_role, recipe.panic AS recipe_panic, recipe._partitions AS recipe__partitions \nFROM recipe INNER JOIN machine_recipe ON recipe.id = machine_recipe.id ORDER BY task_status.status ASC, recipe.id DESC \n LIMIT 0, 50' []

OperationalError: (OperationalError) (1054, "Unknown column 'task_result.result' in 'order clause'") u'SELECT recipe.id AS recipe_id, machine_recipe.id AS machine_recipe_id, recipe.recipe_set_id AS recipe_recipe_set_id, recipe.distro_id AS recipe_distro_id, recipe.system_id AS recipe_system_id, recipe.result_id AS recipe_result_id, recipe.status_id AS recipe_status_id, recipe.start_time AS recipe_start_time, recipe.finish_time AS recipe_finish_time, recipe._host_requires AS recipe__host_requires, recipe._distro_requires AS recipe__distro_requires, recipe.kickstart AS recipe_kickstart, recipe.type AS recipe_type, recipe.ttasks AS recipe_ttasks, recipe.ptasks AS recipe_ptasks, recipe.wtasks AS recipe_wtasks, recipe.ftasks AS recipe_ftasks, recipe.ktasks AS recipe_ktasks, recipe.whiteboard AS recipe_whiteboard, recipe.ks_meta AS recipe_ks_meta, recipe.kernel_options AS recipe_kernel_options, recipe.kernel_options_post AS recipe_kernel_options_post, recipe.role AS recipe_role, recipe.panic AS recipe_panic, recipe._partitions AS recipe__partitions \nFROM recipe INNER JOIN machine_recipe ON recipe.id = machine_recipe.id ORDER BY task_result.result ASC, recipe.id DESC \n LIMIT 0, 50' []

Comment 1 Dan Callaghan 2010-09-02 01:11:56 UTC
It seems SQLAlchemy isn't clever enough to figure out the right joins when ordering by a related property. The datagrid widget is essentially doing this:

    >>> print Recipe.query().order_by(task_status_table.c.status)
    SELECT ...
    FROM recipe ORDER BY task_status.status

which is missing the join.

The best we can do is probably just to join explicitly on all possible sortable relations before we pass the query through to the datagrid widget. That seems to be what we do for the distro datagrid (which sorts related properties correctly):

    def index(self,*args,**kw):
        return self.distros(distros=session.query(Distro).join('breed').join('arch').join(['osversion','osmajor']),*args,**kw)

Comment 2 Dan Callaghan 2010-09-02 01:41:27 UTC
Sorting recipes by arch, distro, or system is also broken (doesn't raise an exception, but doesn't sort anything).

Comment 3 Dan Callaghan 2010-09-02 07:20:35 UTC
Pushed a fix in branch bz629147 for inclusion in 0.5.57:
http://git.fedorahosted.org/git/?p=beaker.git;a=commitdiff;h=28d8af43f68a4801fe67e3fd414769005b9ba42b

I'm still working on a Selenium test for this, will finish that off next week when I return.

Comment 4 Bill Peck 2010-09-02 12:57:47 UTC
Thanks Dan,

This is annoying since I would expect at least this to work.

>>> print Recipe.query().order_by(Recipe.status)
Traceback (most recent call last):
  File "<console>", line 1, in ?
  File "/usr/lib/python2.4/site-packages/sqlalchemy/orm/query.py", line 1254, in __str__
    return str(self.compile())
  File "/usr/lib/python2.4/site-packages/sqlalchemy/orm/query.py", line 1130, in compile
    return self._compile_context().statement
  File "/usr/lib/python2.4/site-packages/sqlalchemy/orm/query.py", line 1199, in _compile_context
    context.order_by = [expression._literal_as_text(o) for o in util.to_list(context.order_by) or []]
  File "/usr/lib/python2.4/site-packages/sqlalchemy/sql/expression.py", line 879, in _literal_as_text
    return element.expression_element()
  File "/usr/lib/python2.4/site-packages/sqlalchemy/orm/attributes.py", line 53, in expression_element
    return self.comparator.expression_element()
  File "/usr/lib/python2.4/site-packages/sqlalchemy/orm/interfaces.py", line 432, in expression_element
    return self.clause_element()
  File "/usr/lib/python2.4/site-packages/sqlalchemy/sql/expression.py", line 1178, in clause_element
    raise NotImplementedError()
NotImplementedError


I can understand the order_by which uses task_status_table.c.status not working but you would think referencing it via the relation attribute it would since it has all the data needed.

Comment 5 Dan Callaghan 2010-09-06 06:05:14 UTC
Bill: agreed, this seems like an sqlalchemy flaw. I wonder if it might be fixed in later versions.

I've written some selenium tests that trigger this bug. These will be included in the selenium test suite which Ray and I are working on getting into beaker's source (eventually). I think this fix is ready to go for 0.5.57.


Note You need to log in before you can comment on or make changes to this bug.