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.

Sign up for GitHub

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

Jump to bottom

SQL: ORDER BY COUNT(*) should display an error message #34590

Closed
astefan opened this issue Oct 18, 2018 · 1 comment · Fixed by #34855
Closed

SQL: ORDER BY COUNT(*) should display an error message #34590

astefan opened this issue Oct 18, 2018 · 1 comment · Fixed by #34855
Assignees
@matriv
Labels
:Analytics/SQL SQL querying >bug

Comments

@astefan
Copy link
Contributor

astefan commented Oct 18, 2018

The following behavior should return an error message about the limitations of ORDER BY, but instead is returning some seemingly incorrect results:

sql> select languages,count(*) from test_emp group by languages having count(*) > 10 order by count(emp_no);
   languages   |   COUNT(1)
---------------+---------------
1              |15
2              |19
3              |17
4              |18
5              |21

composite aggregation (what is used in any group by sql statements) cannot order on something else other than the key itself (in this case - languages). But, instead of returning an error message, it gives the false impression that the results are incorrect.

Related to #31860.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@matriv matriv self-assigned this Oct 25, 2018
matriv added a commit to matriv/elasticsearch that referenced this issue Oct 25, 2018
@matriv
SQL: Return error with ORDER BY on non-grouped.
48720bb 
Previously, for some queries the validation for ORDER BY
fields didn't kick in since a HAVING close or an ORDER BY
with scalar function would add `Filter` and `Project` plans
between the `OrderBy` and the `Aggregate`.

Now the LogicalPlan tree beneath `OrderBy` is traversed and
the ORDER BY fields are properly verified.

Fixes: elastic#34590
@matriv matriv mentioned this issue Oct 25, 2018
Merged
matriv pushed a commit that referenced this issue Oct 25, 2018
SQL: Return error with ORDER BY on non-grouped. (#34855)
06d7431 
Previously, for some queries the validation for ORDER BY
fields didn't kick in since a HAVING close or an ORDER BY
with scalar function would add `Filter` and `Project` plans
between the `OrderBy` and the `Aggregate`.

Now the LogicalPlan tree beneath `OrderBy` is traversed and
the ORDER BY fields are properly verified.

Fixes: #34590
matriv pushed a commit that referenced this issue Oct 25, 2018
@matriv
SQL: Return error with ORDER BY on non-grouped. (#34855)
024116f 
Previously, for some queries the validation for ORDER BY
fields didn't kick in since a HAVING close or an ORDER BY
with scalar function would add `Filter` and `Project` plans
between the `OrderBy` and the `Aggregate`.

Now the LogicalPlan tree beneath `OrderBy` is traversed and
the ORDER BY fields are properly verified.

Fixes: #34590
matriv pushed a commit that referenced this issue Oct 25, 2018
@matriv
SQL: Return error with ORDER BY on non-grouped. (#34855)
fa3abe3 
Previously, for some queries the validation for ORDER BY
fields didn't kick in since a HAVING close or an ORDER BY
with scalar function would add `Filter` and `Project` plans
between the `OrderBy` and the `Aggregate`.

Now the LogicalPlan tree beneath `OrderBy` is traversed and
the ORDER BY fields are properly verified.

Fixes: #34590
kcm pushed a commit that referenced this issue Oct 30, 2018
@kcm
SQL: Return error with ORDER BY on non-grouped. (#34855)
e420b78 
Previously, for some queries the validation for ORDER BY
fields didn't kick in since a HAVING close or an ORDER BY
with scalar function would add `Filter` and `Project` plans
between the `OrderBy` and the `Aggregate`.

Now the LogicalPlan tree beneath `OrderBy` is traversed and
the ORDER BY fields are properly verified.

Fixes: #34590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matriv matriv

Labels
:Analytics/SQL SQL querying >bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SQL: Return error with ORDER BY on non-grouped. matriv/elasticsearch
3 participants
@astefan @matriv @elasticmachine