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

expression in ORDER BY clause does match any expression in the SELECT clause (SQL Server) #5470

Closed
theosotr opened this issue Jul 21, 2020 · 12 comments
Labels
bug Something isn't working sql
Milestone

Comments

@theosotr
Copy link
Contributor

Describe the bug

I run the following query on SQL Server

expr = (Model.columnA.op('+')(2)).label('label')
session.query(expr).select_from(Model).order_by(desc(expr))).distinct()

SQLalchemy generates the following query

SELECT DISTINCT model.columnA + ? AS label, (model.columnA + ?) AS anon_1
FROM model ORDER BY (model.columnA + ?) DESC

the query above crashes on SQL Server with

sqlalchemy.exc.ProgrammingError: (pyodbc.ProgrammingError) ('42000', '[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]ORDER BY items must appear in the select list if SELECT DISTINCT is specified. (145) (SQLExecDirectW)')

Expected behavior

I expect SQLAlchemy to produce this query instead

SELECT DISTINCT model.columnA + ? AS label
FROM model ORDER BY label DESC

Notably, when i replace the op('+') method, with the Python operator + SQLalchemy produces the expected query. That is

expr = (Model.columnA + 2).label('label')
session.query(expr).select_from(Model).order_by(desc(expr))).distinct()

Have a nice day!

@theosotr theosotr added the requires triage New issue that requires categorization label Jul 21, 2020
@zzzeek
Copy link
Member

zzzeek commented Jul 21, 2020

any reason you have to use op("+")? there is a mechanism at play that I would have to review in order to describe how it makes its decisions.

@zzzeek zzzeek changed the title expression in GROUP BY clause does match any expression in the SELECT clause (SQL Server) expression in ORDER BY clause does match any expression in the SELECT clause (SQL Server) Jul 21, 2020
@zzzeek
Copy link
Member

zzzeek commented Jul 21, 2020

hi can't reproduce locally please share the relevant parts of "Model", e.g. what datattype for columnA as well as requested SQLAlchemy version in use thanks

@zzzeek zzzeek added the awaiting info waiting for the submitter to give more information label Jul 21, 2020
@theosotr
Copy link
Contributor Author

Because op() is the generic mechanism for producing arbitrary operators. This test case is simplified for opening the issue.

@theosotr
Copy link
Contributor Author

Models

  class Model(Base):
    __tablename__ = 'model'
    id = Column(Integer, primary_key=True)
    columnA = Column(Numeric(10, 2))

SQLAlchemy version that I used: master
But this also fails on 1.3.

@zzzeek zzzeek added bug Something isn't working sql and removed awaiting info waiting for the submitter to give more information labels Jul 21, 2020
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Allow Grouping to pass along proxy_set of element https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2103

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_3 branch:

Allow Grouping to pass along proxy_set of element https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2104

@zzzeek zzzeek removed the requires triage New issue that requires categorization label Jul 21, 2020
@zzzeek zzzeek added this to the 1.3.x milestone Jul 21, 2020
sqlalchemy-bot pushed a commit that referenced this issue Jul 23, 2020
Repaired an issue where the "ORDER BY" clause rendering a label name rather
than a complete expression, which is particularly important for SQL Server,
would fail to occur if the expression were enclosed in a parenthesized
grouping in some cases.   This case has been added to test support.

Fixes: #5470
Change-Id: Ie0e27c39e5d53be78b32f7810f93d2d0536375e7
(cherry picked from commit 30ec982)
@zzzeek
Copy link
Member

zzzeek commented Aug 13, 2020

the query above still does not work in 1.3 due to the distinct logic done by Query, seems to not happen in 1.4:

SELECT DISTINCT a.col + ? AS label, (a.col + ?) AS label 
FROM a ORDER BY label DESC

additionally the variant from #5511 doesn't work on either. test for both below

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.declarative import declared_attr

Base = declarative_base()


class Yangon(Base):
    __tablename__ = "a"

    id = Column(Integer, primary_key=True)
    col = Column(Integer)


e = create_engine(
    "mssql+pyodbc://scott:tiger^5HHH@mssql2017:1433/test?driver=ODBC+Driver+13+for+SQL+Server",
    echo=True,
)
Base.metadata.drop_all(e)
Base.metadata.create_all(e)

s = Session(e)


def test_one():
    expr = (Yangon.col.op("+")(2)).label("label")
    s.query(expr).select_from(Yangon).order_by(desc(expr)).distinct().all()


def test_two():
    expr = Yangon.col + literal(1)
    s.query(expr).select_from(Yangon).order_by(asc(expr)).distinct().all()

@zzzeek zzzeek reopened this Aug 13, 2020
@zzzeek
Copy link
Member

zzzeek commented Aug 13, 2020

The recommended approach to this problem in all cases is to name the label explicitly in your ORDER BY. this will work on all versions, please use this approach:

expr = (Yangon.col + literal(1)).label("foo")
q = s.query(expr).select_from(Yangon).order_by(asc("foo")).distinct()
q.all()

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Further fixes for ticket 5470 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2146

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_3 branch:

Further fixes for ticket 5470 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2147

sqlalchemy-bot pushed a commit that referenced this issue Aug 13, 2020
The fix for #5470 didn't actually take into account that
the "distinct" logic in query was also doubling up the criteria.
Added many more tests.   the 1.3 version here will be different
than 1.4 as the regression is not quite the same.

Fixes: #5470
Change-Id: I16a23917cab175761de9c867d9d9ac55031d9b97
(cherry picked from commit 7ac2d5c93b6334528aff93939559eee133f3e9fc)
@zzzeek
Copy link
Member

zzzeek commented Aug 13, 2020

OK that's more fixes. you need to use label(), it won't do the "anon_1" part of it. but all the label() cases should work now.

@theosotr
Copy link
Contributor Author

Thank you for your prompt fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sql
Projects
None yet
Development

No branches or pull requests

3 participants