[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Tested-by: Impala Public Jenkins --- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 11 Jul 2018 05:09:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 11 Jul 2018 01:48:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 11 Jul 2018 01:48:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 11 Jul 2018 01:44:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 10 Jul 2018 06:54:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 10 Jul 2018 03:47:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 10 Jul 2018 03:47:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 10 Jul 2018 03:45:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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 Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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
Gerrit-Reviewer: Taras Bobrovytsky
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Mon, 09 Jul 2018 22:12:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7260: Fix decimal binary predicates
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
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Mon, 09 Jul 2018 15:45:53 +
Gerrit-HasComments: Yes
[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
