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: Remove exception throwing from Analyzer #38208

Closed
costin opened this issue Feb 1, 2019 · 1 comment · Fixed by #38260
Closed

SQL: Remove exception throwing from Analyzer #38208

costin opened this issue Feb 1, 2019 · 1 comment · Fixed by #38260
Labels
:Analytics/SQL SQL querying >bug >enhancement v6.6.1 v6.7.0 v7.0.0-beta1

Comments

@costin
Copy link
Member

costin commented Feb 1, 2019

Currently, there's still a rule that uses exceptions in case of problem - while this achieves the goal, the correct way is to let the Verifier collect all issues and report them in one go.
Additionally the same rule produces an incorrect message:

sql> select gender as g, min(salary) as min, max(salary) as max from test_emp group by 1 order by 2, 3, 2, 4 desc limit 3;
Bad request [line 1:103: Invalid %d specified in OrderBy (valid range is [1, %d])]

Which needs fixing.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin costin mentioned this issue Feb 1, 2019
Merged
costin added a commit to costin/elasticsearch that referenced this issue Feb 2, 2019
@costin
SQL: Remove exceptions from Analyzer
7961bf7 
Instead of throwing an exception, use an unresolved attribute to pass
the message to the Verifier.
Additionally improve the parser to save the extended source for the
Aggregate and OrderBy.

Close elastic#38208
@costin costin mentioned this issue Feb 2, 2019
Merged
costin added a commit that referenced this issue Feb 3, 2019
@costin
SQL: Remove exceptions from Analyzer (#38260)
75f0750 
Instead of throwing an exception, use an unresolved attribute to pass
the message to the Verifier.
Additionally improve the parser to save the extended source for the
Aggregate and OrderBy.

Close #38208
costin added a commit to costin/elasticsearch that referenced this issue Feb 3, 2019
@costin
SQL: Remove exceptions from Analyzer (elastic#38260)
16bd376 
Instead of throwing an exception, use an unresolved attribute to pass
the message to the Verifier.
Additionally improve the parser to save the extended source for the
Aggregate and OrderBy.

Close elastic#38208

(cherry picked from commit 75f0750)
@costin costin mentioned this issue Feb 3, 2019
Merged
costin added a commit that referenced this issue Feb 4, 2019
@costin
SQL: Remove exceptions from Analyzer (#38260) (#38287)
21f2c03 
Instead of throwing an exception, use an unresolved attribute to pass
the message to the Verifier.
Additionally improve the parser to save the extended source for the
Aggregate and OrderBy.

Close #38208

(cherry picked from commit 75f0750)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Analytics/SQL SQL querying >bug >enhancement v6.6.1 v6.7.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SQL: Remove exceptions from Analyzer costin/elasticsearch
3 participants
@costin @colings86 @elasticmachine