You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org> on 2018/07/07 01:30:27 UTC
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Taras Bobrovytsky has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10888
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 14 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/10888/1
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2790/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:48:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 3: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2784/
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 06:54:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Reviewed-on: http://gerrit.cloudera.org:8080/10888
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/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 23 insertions(+), 5 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 6
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:48:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@265
PS1, Line 265: AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as bigint)");
> add a comment explaining what this is trying to test.
Done
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2418
PS1, Line 2418: testDecimalExpr(decimal_5_5 + " + cast(1 as tinyint)",
> these tests look more specific than the added ones since the return type is
No we don't get lucky here. Arithmetic operations go through a different code path.
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2496
PS1, Line 2496: public void TestDecimalFunctions() throws AnalysisException {
> worth it to add cases here for non-binary predicates?
I don't think so. This patch only affects the behavior of decimal binary predicates.
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Jul 2018 22:12:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 1:
(3 comments)
looks good, just have a couple of questions about testing.
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@265
PS1, Line 265: AnalyzesOk("select cast(1 as decimal(38,37)) " + operator + " cast(2 as bigint)");
add a comment explaining what this is trying to test.
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2418
PS1, Line 2418: testDecimalExpr(decimal_5_5 + " + cast(1 as tinyint)",
these tests look more specific than the added ones since the return type is checked. coincidentally, did we just get unlucky here that the expected return types happened to be the min resolution type as well?
http://gerrit.cloudera.org:8080/#/c/10888/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2496
PS1, Line 2496: public void TestDecimalFunctions() throws AnalysisException {
worth it to add cases here for non-binary predicates?
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Jul 2018 15:45:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 03:45:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 03:47:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 18 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/10888/2
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
IMPALA-7260: Fix decimal binary predicates
When casting the inputs to a function call, we would try to cast non
decimal numbers to a specific decimal type even though this is not
necessary. The specific decimal type would be calculated by looking at
all the inputs. It was possible for this calculation to fail due to some
decimal types being incompatible with each other.
We solve the problem by casting the non-decimal numerical inputs to a
generic getMinResolutionDecimal().
Testing:
- Added some analysis tests.
Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 23 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/10888/4
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 05:09:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2784/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 3
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 03:47:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10888 )
Change subject: IMPALA-7260: Fix decimal binary predicates
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc442e84cdff74a376ba25bd9897b0b4df5cf0b4
Gerrit-Change-Number: 10888
Gerrit-PatchSet: 4
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:44:33 +0000
Gerrit-HasComments: No