.. changelog::
:version: 1.0.3
+ .. change::
+ :tags: bug, orm
+ :tickets: 3392
+
+ Fixed regression due to :ticket:`2992` where textual elements placed
+ into the :meth:`.Query.order_by` clause in conjunction with joined
+ eager loading would be added to the columns clause of the inner query
+ in such a way that they were assumed to be table-bound column names,
+ in the case where the joined eager load needs to wrap the query
+ in a subquery to accommodate for a limit/offset.
+
+ Originally, the behavior here was intentional, in that a query such
+ as ``query(User).order_by('name').limit(1)``
+ would order by ``user.name`` even if the query was modified by
+ joined eager loading to be within a subquery, as ``'name'`` would
+ be interpreted as a symbol to be located within the FROM clauses,
+ in this case ``User.name``, which would then be copied into the
+ columns clause to ensure it were present for ORDER BY. However, the
+ feature fails to anticipate the case where ``order_by("name")`` refers
+ to a specific label name present in the local columns clause already
+ and not a name bound to a selectable in the FROM clause.
+
+ Beyond that, the feature also fails for deprecated cases such as
+ ``order_by("name desc")``, which, while it emits a
+ warning that :func:`.text` should be used here (note that the issue
+ does not impact cases where :func:`.text` is used explicitly),
+ still produces a different query than previously where the "name desc"
+ expression is copied into the columns clause inappropriately. The
+ resolution is such that the "joined eager loading" aspect of the
+ feature will skip over these so-called "label reference" expressions
+ when augmenting the inner columns clause, as though they were
+ :func:`.text` constructs already.
+
.. change::
:tags: bug, sql
:tickets: 3391
[User(id=7), User(id=8), User(id=9), User(id=10)]
)
- def test_order_by_w_eager(self):
+ def test_order_by_w_eager_one(self):
+ User = self.classes.User
+ s = create_session()
+
+ # from 1.0.0 thru 1.0.2, the "name" symbol here was considered
+ # to be part of the things we need to ORDER BY and it was being
+ # placed into the inner query's columns clause, as part of
+ # query._compound_eager_statement where we add unwrap_order_by()
+ # to the columns clause. However, as #3392 illustrates, unlocatable
+ # string expressions like "name desc" will only fail in this scenario,
+ # so in general the changing of the query structure with string labels
+ # is dangerous.
+ #
+ # the queries here are again "invalid" from a SQL perspective, as the
+ # "name" field isn't matched up to anything.
+ #
+ with expect_warnings("Can't resolve label reference 'name';"):
+ self.assert_compile(
+ s.query(User).options(joinedload("addresses")).
+ order_by(desc("name")).limit(1),
+ "SELECT anon_1.users_id AS anon_1_users_id, "
+ "anon_1.users_name AS anon_1_users_name, "
+ "addresses_1.id AS addresses_1_id, "
+ "addresses_1.user_id AS addresses_1_user_id, "
+ "addresses_1.email_address AS addresses_1_email_address "
+ "FROM (SELECT users.id AS users_id, users.name AS users_name "
+ "FROM users ORDER BY users.name "
+ "DESC LIMIT :param_1) AS anon_1 "
+ "LEFT OUTER JOIN addresses AS addresses_1 "
+ "ON anon_1.users_id = addresses_1.user_id "
+ "ORDER BY name DESC, addresses_1.id"
+ )
+
+ def test_order_by_w_eager_two(self):
+ User = self.classes.User
+ s = create_session()
+
+ with expect_warnings("Can't resolve label reference 'name';"):
+ self.assert_compile(
+ s.query(User).options(joinedload("addresses")).
+ order_by("name").limit(1),
+ "SELECT anon_1.users_id AS anon_1_users_id, "
+ "anon_1.users_name AS anon_1_users_name, "
+ "addresses_1.id AS addresses_1_id, "
+ "addresses_1.user_id AS addresses_1_user_id, "
+ "addresses_1.email_address AS addresses_1_email_address "
+ "FROM (SELECT users.id AS users_id, users.name AS users_name "
+ "FROM users ORDER BY users.name "
+ "LIMIT :param_1) AS anon_1 "
+ "LEFT OUTER JOIN addresses AS addresses_1 "
+ "ON anon_1.users_id = addresses_1.user_id "
+ "ORDER BY name, addresses_1.id"
+ )
+
+ def test_order_by_w_eager_three(self):
+ User = self.classes.User
+ s = create_session()
+
+ self.assert_compile(
+ s.query(User).options(joinedload("addresses")).
+ order_by("users_name").limit(1),
+ "SELECT anon_1.users_id AS anon_1_users_id, "
+ "anon_1.users_name AS anon_1_users_name, "
+ "addresses_1.id AS addresses_1_id, "
+ "addresses_1.user_id AS addresses_1_user_id, "
+ "addresses_1.email_address AS addresses_1_email_address "
+ "FROM (SELECT users.id AS users_id, users.name AS users_name "
+ "FROM users ORDER BY users.name "
+ "LIMIT :param_1) AS anon_1 "
+ "LEFT OUTER JOIN addresses AS addresses_1 "
+ "ON anon_1.users_id = addresses_1.user_id "
+ "ORDER BY anon_1.users_name, addresses_1.id"
+ )
+
+ # however! this works (again?)
+ eq_(
+ s.query(User).options(joinedload("addresses")).
+ order_by("users_name").first(),
+ User(name='chuck', addresses=[])
+ )
+
+ def test_order_by_w_eager_four(self):
User = self.classes.User
Address = self.classes.Address
s = create_session()
- # here, we are seeing how Query has to take the order by expressions
- # of the query and then add them to the columns list, so that the
- # outer subquery can order by that same label. With the anonymous
- # label, our column gets sucked up and restated again in the
- # inner columns list!
- # we could try to play games with making this "smarter" but it
- # would add permanent overhead to Select._columns_plus_names,
- # since that's where references would need to be resolved.
- # so as it is, this query takes the _label_reference and makes a
- # full blown proxy and all the rest of it.
self.assert_compile(
s.query(User).options(joinedload("addresses")).
- order_by(desc("name")).limit(1),
+ order_by(desc("users_name")).limit(1),
"SELECT anon_1.users_id AS anon_1_users_id, "
"anon_1.users_name AS anon_1_users_name, "
- "anon_1.anon_2 AS anon_1_anon_2, "
"addresses_1.id AS addresses_1_id, "
"addresses_1.user_id AS addresses_1_user_id, "
"addresses_1.email_address AS addresses_1_email_address "
- "FROM (SELECT users.id AS users_id, users.name AS users_name, "
- "users.name AS anon_2 FROM users ORDER BY users.name "
- "DESC LIMIT :param_1) AS anon_1 "
+ "FROM (SELECT users.id AS users_id, users.name AS users_name "
+ "FROM users ORDER BY users.name DESC "
+ "LIMIT :param_1) AS anon_1 "
"LEFT OUTER JOIN addresses AS addresses_1 "
"ON anon_1.users_id = addresses_1.user_id "
- "ORDER BY anon_1.anon_2 DESC, addresses_1.id"
+ "ORDER BY anon_1.users_name DESC, addresses_1.id"
)
+ # however! this works (again?)
eq_(
s.query(User).options(joinedload("addresses")).
- order_by(desc("name")).first(),
+ order_by(desc("users_name")).first(),
User(name='jack', addresses=[Address()])
)
+ def test_order_by_w_eager_five(self):
+ """essentially the same as test_eager_relations -> test_limit_3,
+ but test for textual label elements that are freeform.
+ this is again #3392."""
+
+ User = self.classes.User
+ Address = self.classes.Address
+ Order = self.classes.Order
+
+ sess = create_session()
+
+ q = sess.query(User, Address.email_address.label('email_address'))
+
+ l = q.join('addresses').options(joinedload(User.orders)).\
+ order_by(
+ "email_address desc").limit(1).offset(0)
+ with expect_warnings(
+ "Can't resolve label reference 'email_address desc'"):
+ eq_(
+ [
+ (User(
+ id=7,
+ orders=[Order(id=1), Order(id=3), Order(id=5)],
+ addresses=[Address(id=1)]
+ ), 'jack@bean.com')
+ ],
+ l.all())
+
class TextWarningTest(QueryTest, AssertsCompiledSQL):
def _test(self, fn, arg, offending_clause, expected):
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
