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

SQL: selecting a literal from grouped by query generates error #41413

Closed
astefan opened this issue Apr 22, 2019 · 15 comments
Closed

SQL: selecting a literal from grouped by query generates error #41413

astefan opened this issue Apr 22, 2019 · 15 comments
Assignees
Labels

Comments

@astefan
Copy link
Contributor

astefan commented Apr 22, 2019

sql> select 1 from test_emp group by gender;
Bad request [Found 1 problem(s)
line 1:33: Unknown column [gender], did you mean [gender]?]
@astefan astefan self-assigned this Apr 22, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@codebird
Copy link
Contributor

codebird commented May 6, 2019

I was thinking of a solution like this:

https://github.com/elastic/elasticsearch/compare/master...codebird:issue%2341413?expand=1

Logic behind it, is that if all we're selecting are literals, then resolved should become all acceptable fields in our field list. Now for some reason, this is considering the group by field as a literal too. so this:

select 1 from testing group by name

is being considered like this:

select 1 from testing group by 'name'

So I wanted to get some input on my approach before going forward.

@matriv
Copy link
Contributor

matriv commented May 9, 2019

@astefan this is similar to #34583, Isn't it? Should we close the old one as a duplicate?

@astefan
Copy link
Contributor Author

astefan commented May 9, 2019

@codebird thank you for looking at this. We haven't yet started looking at the issue or the proposed PR.

Not sure @matriv, but I'd be cautious and keep both. There are other issues around literals in different scenarios, as well.

@matriv
Copy link
Contributor

matriv commented Feb 4, 2020

The status of such queries has been changed. No error is thrown but instead only one row is returned, independently of the buckets that the group by would return, since they are wrongly translated to LocalExec plans.

@codebird
Copy link
Contributor

codebird commented Feb 4, 2020

my pull request solves this issue, but for some reason it has been forgotten.

@matriv
Copy link
Contributor

matriv commented Feb 4, 2020

Hi @codebirg,

Apologies for the long delay, but we didn't forget your PR. We wanted to wait for the refactoring merged with: #49570 which simplifies the code in many aspects, so that we could address such issues in a less "hacky" way.

@codebird
Copy link
Contributor

codebird commented Feb 5, 2020

I'll merge into my branch and check it out @matriv will try to check it out this week

@matriv
Copy link
Contributor

matriv commented Feb 5, 2020

Thank you @codebird !

@codebird
Copy link
Contributor

codebird commented Feb 5, 2020

@matriv all issue were already fixed, except #41951, and aliasing a literal

Ex:
SELECT COUNT(*) as c FROM test WHERE ABS(int) > 0 GROUP BY int
SELECT 1 as t FROM test GROUP BY int

I updated my pull request accordingly...

@matriv
Copy link
Contributor

matriv commented Feb 5, 2020

@codebird
SELECT 1 as t FROM test GROUP BY int
is not fixed. It doesn't throw an Exception but it returns only one row, where it should return one row per group (created by values of int in this case)

@codebird
Copy link
Contributor

codebird commented Feb 6, 2020

thank you for looking into it @matriv, this slipped through I had it fixed this time, seems I forgot to move some of the code. I'll fix it today and start working on tests.

@codebird
Copy link
Contributor

codebird commented Feb 7, 2020

@matriv this issue SELECT 1 as t FROM test GROUP BY int is also on this query SELECT 1 FROM test GROUP BY int, I am still trying to figure out, but notice that both queries return the same result. SELECT 1 FROM test GROUP BY int <- this is currently on master.

@codebird
Copy link
Contributor

codebird commented Feb 7, 2020

the issue with these 2 queries was fixed. If you can check out my solution. I'll go on with writing the tests.

@matriv matriv closed this as completed in 45b8580 Feb 15, 2020
matriv pushed a commit that referenced this issue Feb 15, 2020
Translate to an agg query even if only literals are selected,
so that the correct number of rows is returned (number of buckets).

Fix issue with key only in GROUP BY (not in select) and WHERE clause:
Resolve aggregates and groupings based on the child plan which holds
the info info for all the fields of the underlying table.

Fixes: #41951
Fixes: #41413
(cherry picked from commit 45b8580)
matriv pushed a commit that referenced this issue Feb 15, 2020
Translate to an agg query even if only literals are selected,
so that the correct number of rows is returned (number of buckets).

Fix issue with key only in GROUP BY (not in select) and WHERE clause:
Resolve aggregates and groupings based on the child plan which holds
the info info for all the fields of the underlying table.

Fixes: #41951
Fixes: #41413
(cherry picked from commit 45b8580)
@matriv
Copy link
Contributor

matriv commented Feb 15, 2020

master : 45b8580
7.x : 5b32d11
7.6 : 967f702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants