You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/04/01 14:43:15 UTC
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Hello Vuk Ercegovac, Impala Public Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/12902
to review the following change.
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
IMPALA-7211: Fix the between predicate for decimals
Before this patch, some queries would fail where the inputs to the
between predicate were decimal types that are not compatible with each
other. We would needlessly cast them to the same type in the FE. This
was not necessary because we are able to handle decimals of different
types in the backend already for this predicate. This patch should even
result in a slight performance improvement.
Testing:
- Added some tests to AnalyzeExprsTest.
Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Reviewed-on: http://gerrit.cloudera.org:8080/10898
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
3 files changed, 66 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/12902/3
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3971/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 18:54:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 15:21:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2605/ : 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/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2019 15:23:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
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/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
IMPALA-7211: Fix the between predicate for decimals
Before this patch, some queries would fail where the inputs to the
between predicate were decimal types that are not compatible with each
other. We would needlessly cast them to the same type in the FE. This
was not necessary because we are able to handle decimals of different
types in the backend already for this predicate. This patch should even
result in a slight performance improvement.
Testing:
- Added some tests to AnalyzeExprsTest.
Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Reviewed-on: http://gerrit.cloudera.org:8080/10898
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/12902
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
3 files changed, 66 insertions(+), 63 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:55:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7211: Fix the between predicate for decimals
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12902 )
Change subject: IMPALA-7211: Fix the between predicate for decimals
......................................................................
Patch Set 3:
Cherry-pick conflicts: fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
How to resolve the conflicts:
We should explictly enable decimal_v2 in decimal cast relative tests so AnalysisError and AnalysisOk can work as expected.
--
To view, visit http://gerrit.cloudera.org:8080/12902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac89d62082052b41bfa698e4de09aec26df5c312
Gerrit-Change-Number: 12902
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Apr 2019 14:47:29 +0000
Gerrit-HasComments: No