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: CAST and CONVERT sporadically generate errors when grouped by #40240

Closed
astefan opened this issue Mar 20, 2019 · 2 comments · Fixed by #43072
Closed

SQL: CAST and CONVERT sporadically generate errors when grouped by #40240

astefan opened this issue Mar 20, 2019 · 2 comments · Fixed by #43072
Assignees
Labels

Comments

@astefan
Copy link
Contributor

astefan commented Mar 20, 2019

sql> SELECT CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) FROM test_emp GROUP BY CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT);
Server error [Server sent bad type [folding_exception]. Original type was [line 1:8: Cannot find grouping for 'CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT)']. [FoldingException[line 1:8: Cannot find grouping for 'CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT)']
        at org.elasticsearch.xpack.sql.planner.QueryFolder$FoldAggregate.rule(QueryFolder.java:321)
        at org.elasticsearch.xpack.sql.planner.QueryFolder$FoldAggregate.rule(QueryFolder.java:206)
        at org.elasticsearch.xpack.sql.tree.Node.lambda$transformUp$11(Node.java:196)
        at org.elasticsearch.xpack.sql.tree.Node.transformUp(Node.java:190)
        at org.elasticsearch.xpack.sql.tree.Node.transformUp(Node.java:196)

And same goes for CAST to the same data type:

sql> SELECT CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) FROM test_emp GROUP BY CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT);
Server error [Server sent bad type [folding_exception]. Original type was [line 1:8: Cannot find grouping for 'CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT)']. [FoldingException[line 1:8: Cannot find grouping for 'CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT)']
        at org.elasticsearch.xpack.sql.planner.QueryFolder$FoldAggregate.rule(QueryFolder.java:321)
        at org.elasticsearch.xpack.sql.planner.QueryFolder$FoldAggregate.rule(QueryFolder.java:206)
        at org.elasticsearch.xpack.sql.tree.Node.lambda$transformUp$11(Node.java:196)
        at org.elasticsearch.xpack.sql.tree.Node.transformUp(Node.java:190)

BUT, same query with a different type to be casted to works:

sql> SELECT CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) FROM test_emp GROUP BY CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS INTEGER);
ABS(EXTRACT(YEAR FROM "birth_date"))                                                                                                           
------------------------------------                                                                                                           
null                                                                                                                                           
1952                                                                                                                                           
1953                                                                                                                                           
1954                                                                                                                                           
1955                                                                                                                                           
1956                                                                                                                                           
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin
Copy link
Member

costin commented Mar 25, 2019

The issue here is related to function identity and the reuse of ExpressionId:

  1. since CAST is a keyword, the parser creates the function for it directly without having to resolve it. Since there are two declarations, there are two different functions ids which differ causing the folder to not find the match.
    A potential solution here is to find the duplicate functions and replace it however this needs to take into account any possible reference to either id.

  2. the second query works since the cast is removed/pruned since it has no effect (it's an int being casted to another int).

matriv pushed a commit that referenced this issue Oct 31, 2019
Fix an issue that arises from the use of ExpressionIds as keys in a lookup map
that helps the QueryTranslator to identify the grouping columns. The issue is
that the same expression in different parts of the query (SELECT clause and GROUP BY clause)
ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds
use the hashCode() of NamedExpression.

Fixes: #41159
Fixes: #40001
Fixes: #40240
Fixes: #33361
Fixes: #46316
Fixes: #36074
Fixes: #34543
Fixes: #37044

Fixes: #42041
matriv pushed a commit that referenced this issue Oct 31, 2019
Fix an issue that arises from the use of ExpressionIds as keys in a lookup map
that helps the QueryTranslator to identify the grouping columns. The issue is
that the same expression in different parts of the query (SELECT clause and GROUP BY clause)
ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds
use the hashCode() of NamedExpression.

Fixes: #41159
Fixes: #40001
Fixes: #40240
Fixes: #33361
Fixes: #46316
Fixes: #36074
Fixes: #34543
Fixes: #37044

Fixes: #42041
(cherry picked from commit 3c38ea5)
matriv pushed a commit that referenced this issue Oct 31, 2019
Fix an issue that arises from the use of ExpressionIds as keys in a lookup map
that helps the QueryTranslator to identify the grouping columns. The issue is
that the same expression in different parts of the query (SELECT clause and GROUP BY clause)
ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds
use the hashCode() of NamedExpression.

Fixes: #41159
Fixes: #40001
Fixes: #40240
Fixes: #33361
Fixes: #46316
Fixes: #36074
Fixes: #34543
Fixes: #37044

Fixes: #42041
(cherry picked from commit 3c38ea5)
matriv pushed a commit that referenced this issue Oct 31, 2019
Fix an issue that arises from the use of ExpressionIds as keys in a lookup map
that helps the QueryTranslator to identify the grouping columns. The issue is
that the same expression in different parts of the query (SELECT clause and GROUP BY clause)
ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds
use the hashCode() of NamedExpression.

Fixes: #41159
Fixes: #40001
Fixes: #40240
Fixes: #33361
Fixes: #46316
Fixes: #36074
Fixes: #34543
Fixes: #37044

Fixes: #42041
(cherry picked from commit 3c38ea5)
matriv pushed a commit that referenced this issue Oct 31, 2019
Fix an issue that arises from the use of ExpressionIds as keys in a lookup map
that helps the QueryTranslator to identify the grouping columns. The issue is
that the same expression in different parts of the query (SELECT clause and GROUP BY clause)
ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds
use the hashCode() of NamedExpression.

Fixes: #41159
Fixes: #40001
Fixes: #40240
Fixes: #33361
Fixes: #46316
Fixes: #36074
Fixes: #34543
Fixes: #37044

Fixes: #42041
(cherry picked from commit 3c38ea5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants