eagerload + eagerload subq + subquery attribute + order by the subquery, inappropriately breaks out elements of the subquery into the select

from sqlalchemy import *
from sqlalchemy.orm import *

metadata = MetaData()

books_table = Table('books', metadata,
    Column('id', Integer, primary_key=True)
)

purchases_table = Table('purchases', metadata,
    Column('id', Integer, primary_key=True),
    Column('book_id', Integer, ForeignKey('books.id')),
    Column('quantity', Integer),
)

class Book(object):
    def __init__(self, purchases):
        self.purchases=purchases

class Purchase(object):
    def __init__(self, quantity):
        self.quantity = quantity

mapper(Book, books_table, properties={
    'num_purchased': column_property(
        select((func.sum(purchases_table.c.quantity),)
        ).where(
            books_table.c.id == purchases_table.c.book_id
        )   #  with or without this, produces different error,
            # either no col to correlate, or missing col
            # .correlate(books_table)
    )
})

mapper(Purchase, purchases_table, properties={
    'book': relation(Book, backref='purchases')
})

engine = create_engine('sqlite:///:memory:', echo=True)
metadata.create_all(engine)

session = Session(engine)
session.add_all([   Book(purchases=[Purchase(3), Purchase(5)](
)),
    Book(purchases=[Purchase(9)](Purchase(7),)),
])

session.commit()

results = (
    session.query(Book).options(
                               eagerload_all('purchases')
                           ).
                        order_by(Book.num_purchased).
                           limit(50).
                           all()
)

another version:

cp = select((func.sum(purchases_table.c.quantity),)
        ).where(
            books_table.c.id == purchases_table.c.book_id
        ).correlate(books_table).label('foo')

mapper(Book, books_table)

mapper(Purchase, purchases_table, properties={
    'book': relation(Book, backref='purchases')
})

# ...
results = (
    session.query(Book).options(
                               eagerload_all('purchases')
                           ).
                        order_by(cp).
                           limit(50).
                           all()
)

solution, be more careful when unwrapping the order by inside of _compile_context to not dig into subqueries, labels.

tests should include:

order by subquery
order by scalar subquery
order by label
order by a unary expr that isn't an ASC/DESC, if possible

patch:

diff -r 2f88fd7c6f0250a914675b2fe609be9ed1f72cee lib/sqlalchemy/orm/query.py
--- a/lib/sqlalchemy/orm/query.py   Mon Jun 06 22:24:50 2011 -0400
+++ b/lib/sqlalchemy/orm/query.py   Wed Jun 08 01:47:36 2011 -0400
@@ -2466,7 +2466,7 @@
             if context.order_by:
                 order_by_col_expr = list(
                                         chain(*[                                           sql_util.find_columns(o) 
+                                            sql_util.unwrap_order_by(o) 
                                             for o in context.order_by
                                         ](
-))
                                     )
@@ -2480,6 +2480,9 @@
                         from_obj=froms,
                         use_labels=labels,
                         correlate=False,
+                        # TODO: this order_by is only needed if 
+                        # LIMIT/OFFSET is present in self._select_args,
+                        # else the application on the outside is enough
                         order_by=context.order_by,
                         **self._select_args
                     )
@@ -2527,7 +2530,7 @@
             if self._distinct and context.order_by:
                 order_by_col_expr = list(
                                         chain(*[                                           sql_util.find_columns(o) 
+                                            sql_util.unwrap_order_by(o) 
                                             for o in context.order_by
                                         ](
-))
                                     )
diff -r 2f88fd7c6f0250a914675b2fe609be9ed1f72cee lib/sqlalchemy/sql/operators.py
--- a/lib/sqlalchemy/sql/operators.py   Mon Jun 06 22:24:50 2011 -0400
+++ b/lib/sqlalchemy/sql/operators.py   Wed Jun 08 01:47:36 2011 -0400
@@ -524,6 +524,10 @@
 def is_commutative(op):
     return op in _commutative

+def is_ordering_modifier(op):
+    return op in (asc_op, desc_op, 
+                    nullsfirst_op, nullslast_op)
+
 _associative = _commutative.union([and_, or_](concat_op,))


diff -r 2f88fd7c6f0250a914675b2fe609be9ed1f72cee lib/sqlalchemy/sql/util.py
--- a/lib/sqlalchemy/sql/util.py    Mon Jun 06 22:24:50 2011 -0400
+++ b/lib/sqlalchemy/sql/util.py    Wed Jun 08 01:47:36 2011 -0400
@@ -8,6 +8,7 @@
 from sqlalchemy.util import topological
 from sqlalchemy.sql import expression, operators, visitors
 from itertools import chain
+from collections import deque

 """Utility functions that build upon SQL and Schema constructs."""

@@ -99,6 +100,25 @@
     visitors.traverse(clause, {}, {'column':cols.add})
     return cols

+def unwrap_order_by(clause):
+    """Break up an 'order by' expression into individual column-expressions,
+    without DESC/ASC/NULLS FIRST/NULLS LAST"""
+
+    cols = util.column_set()
+    stack = deque([clause](clause))
+    while stack:
+        t = stack.popleft()
+        if isinstance(t, expression.ColumnElement) and \
+            (
+                not isinstance(t, expression._UnaryExpression) or \
+                not operators.is_ordering_modifier(t.modifier)
+            ): 
+            cols.add(t)
+        else:
+            for c in t.get_children():
+                stack.append(c)
+    return cols
+
 def clause_is_present(clause, search):
     """Given a target clause and a second to search within, return True
     if the target is plainly present in the search without any

Comments (3)