You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2022/10/11 14:09:35 UTC
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19124
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on literal to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding the cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M tests/query_test/test_queries.py
2 files changed, 40 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/19124/1
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Oct 2022 11:22:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on literal to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding the cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M tests/query_test/test_queries.py
2 files changed, 41 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/19124/2
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19124/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19124/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-11462: Constant folding causes cast on literal to be lost
"int literals" could be more exact
http://gerrit.cloudera.org:8080/#/c/19124/3/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/19124/3/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@201
PS3, Line 201: keepType
maybe keepOriginalIntType would be clearer?
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py@385
PS3, Line 385: TestConstantFoldingNoTypeLoss
I think that test_exprs.py or expr-test.cc would be better for this test, as it is about expression handling, not more complex query behavior.
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py@408
PS3, Line 408: query_template = ("select shiftleft(cast(1 as {typename}), z) c "
idea for an extra test without inline view:
select typeof(cast(1 as bigint) + cast(rand() as tinyint));
Currently returns SMALLINT/BIGINT depending on whether expression rewrites are enabled.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 14:27:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11618/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Oct 2022 13:23:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Oct 2022 16:33:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11606/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Oct 2022 14:46:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on literal to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding an explicit cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
M tests/query_test/test_queries.py
3 files changed, 70 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/19124/3
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on int literals to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding an explicit cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
M tests/query_test/test_exprs.py
3 files changed, 80 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/19124/5
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 11:17:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8725/
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 15:32:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8729/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 21 Oct 2022 11:22:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on int literals to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding an explicit cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
M tests/query_test/test_exprs.py
3 files changed, 75 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/19124/4
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11671/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 21:52:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8725/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 11:17:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19124/1/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/19124/1/tests/query_test/test_queries.py@384
PS1, Line 384: class TestConstantFoldingNoTypeLoss(ImpalaTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/19124/1/tests/query_test/test_queries.py@410
PS1, Line 410: -
flake8: E226 missing whitespace around arithmetic operator
http://gerrit.cloudera.org:8080/#/c/19124/1/tests/query_test/test_queries.py@410
PS1, Line 410:
flake8: E261 at least two spaces before inline comment
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Oct 2022 14:10:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on literal to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on literal to be lost
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11604/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Oct 2022 14:30:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
IMPALA-11462: Constant folding causes cast on int literals to be lost
The following query returns -128 instead of 128 and the return type is
TINYINT instead of BIGINT:
select shiftleft(cast(1 as bigint), z) c from (select 7 z ) x;
However, if we disable expression rewrites, the result is correct.
The expression rewrite rule 'FoldConstantsRule' folds the the cast to
bigint into the literal during expression rewrite. This modifies the
expression, so re-analysis is needed. Re-analysis resets the literal
expression (a NumericLiteral), which loses its type and becomes TINYINT
again.
'NumericLiteral's have three kinds of type:
- natural type: the smallest type that can hold its value
- explicit type: starts as the natural type and can be widened by
explicit casts
- implicit type: the result of casts the analyzer uses to
adjust input types for functions and arithmetic operations
See NumericLiteral.java for more.
The problem is that when 'FoldConstantsRule' folds the cast into the
literal, it doesn't set its explicit type (only its implicit type
becomes BIGINT). When the expression is reset during re-analysis, the
type is reverted to the incorrect explicit type.
(In the case where expression rewrites are disabled, no re-analysis
takes place so the literal and its type are not reset.)
This patch fixes the error by setting the explicit type of
'NumericLiteral' when folding an explicit cast into it.
Testing:
- Added test
tests/query_test/test_queries.py::TestConstantFoldingNoTypeLoss::test_shiftleft
that tests 'shiftleft' with all integer types.
Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Reviewed-on: http://gerrit.cloudera.org:8080/19124
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java
M tests/query_test/test_exprs.py
3 files changed, 80 insertions(+), 17 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 8
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19124/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19124/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-11462: Constant folding causes cast on int literals to be lost
> "int literals" could be more exact
Done
http://gerrit.cloudera.org:8080/#/c/19124/3/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/19124/3/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@201
PS3, Line 201: keepOrig
> maybe keepOriginalIntType would be clearer?
Done, also updated in createIntegerLiteral().
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py@385
PS3, Line 385:
> I think that test_exprs.py or expr-test.cc would be better for this test, a
Moved to test_exprs.py.
http://gerrit.cloudera.org:8080/#/c/19124/3/tests/query_test/test_queries.py@408
PS3, Line 408:
> idea for an extra test without inline view:
Created a new test function for it.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 21:33:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19124/4/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/19124/4/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@201
PS4, Line 201: * If 'keepOriginalIntType' is true, the type of the result will be the same as the type of
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/19124/4/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@340
PS4, Line 340: return keepOriginalIntType ? new NumericLiteral(value, type) : new NumericLiteral(value);
line too long (93 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 21:34:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 10:34:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19124/5/tests/query_test/test_exprs.py
File tests/query_test/test_exprs.py:
http://gerrit.cloudera.org:8080/#/c/19124/5/tests/query_test/test_exprs.py@264
PS5, Line 264: ,
flake8: E231 missing whitespace after ','
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 10:43:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11462: Constant folding causes cast on int literals to be lost
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19124 )
Change subject: IMPALA-11462: Constant folding causes cast on int literals to be lost
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11675/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/19124
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7f27b204792ef7c59dec5ead363d44ed0c3bc79
Gerrit-Change-Number: 19124
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 10:21:36 +0000
Gerrit-HasComments: No