ordering by a label present in an `alias` will create an invalid query
this is a new issue with 1.0.0b2 and Postgres. i am not sure if other databases will fail.
this is a bit of an confusing to describe, so bear with me.
i made an example, but it is confusing too!
https://gist.github.com/jvanasco/986c30d91095cef5526f
the issue is in the interplay of aliased columns and an "ORDER BY"
If the ordering column's name is the same as a column name in the aliased "select", then an ordering will be applied to the column in the aliased select -- which breaks a GROUP BY. if the column name is not in the aliased select, then the ordering will be generated as a bareword -- which is the valid query.
-
query_form_a this worked in 0.9.9, it does not work in 1.0.0b2
line 100 applies a 'event_timestamp' label over the 'event_timestamp' column. line 110 sorts on 'event_timestamp'
This will output 0.9.9
ORDER BY event_timestamp DESC
1.0.0b2ORDER BY q_stream.event_timestamp DESC
-
query form b this did not work in 0.9.9 and does not work in 1.0.0b2 just shown for comparison
line 137 we select an 'event_timestamp' column line 147 sorts on 'event_timestamp'
-
query form c this works on 0.9.9 and 1.0.0b2 however it requires a column renaming
line 174 we select the 'event_timestamp' column and rename it 'timestamp_renamed' line 184 sorts on 'timestamp_renamed'
This will output 0.9.9
ORDER BY timestamp_renamed DESC
1.0.0b2ORDER BY timestamp_renamed DESC
note: i used the following sqlalchemy connection string so it would hit the DB and raise the query exceptions for postgres. i found that easier than inspecting the sql in a memory engine. user - sqlalchemy_test password - sqla database - sqlalchemy_test
Comments (8)
-
repo owner -
repo owner also, if you wrap your name in
text("foo")
, then it will work exactly like 0.9. So that might be what we're doing here. -
reporter Ah yes, that's exactly it.
The text() should work. I'll have to analyze all my queries.
-
repo owner there is probably a bug here which is that the "order_by" should be catching the outermost expression.
-
repo owner yup, here's the patch
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 6520f08..b5a7489 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -2680,7 +2680,8 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): only_froms = dict( (c.key, c) for c in _select_iterables(self.froms) if c._allow_label_resolve) - with_cols.update(only_froms) + for key, value in only_froms.items(): + with_cols.setdefault(key, value) return with_cols, only_froms
-
repo owner - changed status to resolved
- Fixed bug in new "label resolution" feature of
2992
where the string label placed in the order_by() or group_by() of a statement would place higher priority on the name as found inside the FROM clause instead of a more locally available name inside the columns clause. fixes#3335
→ <<cset c09c21c9f318>>
-
repo owner this is all set for b4. latest master should resolve your issue.
-
reporter thanks! backwards compatibility FTW!
- Log in to comment
OK, I see form A is basically
desc("some_literal_name")
, the change here is http://docs.sqlalchemy.org/en/latest/changelog/migration_10.html#order-by-and-group-by-are-special-cases. this name is no longer naively dumped into the query its matched to where it most likely belongs.haven't looked at specifics yet but see if that helps explain what you're seeing.