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/03/20 02:04:56 UTC

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Hello Impala Public Jenkins,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/12807

to review the following change.


Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................

IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

In decimal v2, performing a cast that can result in a loss of precision
is considered as an error. In the prior code when finding a compatible
type for performing a cast between expressions that have decimal and
floating types can cause the decimal expressions to never be compared to
other decimal expressions. This results in a cast to a type that may not
be truly compatible in all expressions, especially when decimal v2 is
enabled. This patch fixes it by grouping all the decimals together to make
sure they are compatible with each other.

Testing:
- Added new FE tests
- Ran all core tests

Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Reviewed-on: http://gerrit.cloudera.org:8080/10882
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/Analyzer.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 100 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/12807/2
-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................

IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

In decimal v2, performing a cast that can result in a loss of precision
is considered as an error. In the prior code when finding a compatible
type for performing a cast between expressions that have decimal and
floating types can cause the decimal expressions to never be compared to
other decimal expressions. This results in a cast to a type that may not
be truly compatible in all expressions, especially when decimal v2 is
enabled. This patch fixes it by grouping all the decimals together to make
sure they are compatible with each other.

Testing:
- Added new FE tests
- Ran all core tests

Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Reviewed-on: http://gerrit.cloudera.org:8080/10882
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/12807
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 101 insertions(+), 5 deletions(-)

Approvals:
  Fredy Wijaya: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 17:11:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12807

to look at the new patch set (#3).

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................

IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

In decimal v2, performing a cast that can result in a loss of precision
is considered as an error. In the prior code when finding a compatible
type for performing a cast between expressions that have decimal and
floating types can cause the decimal expressions to never be compared to
other decimal expressions. This results in a cast to a type that may not
be truly compatible in all expressions, especially when decimal v2 is
enabled. This patch fixes it by grouping all the decimals together to make
sure they are compatible with each other.

Testing:
- Added new FE tests
- Ran all core tests

Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Reviewed-on: http://gerrit.cloudera.org:8080/10882
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/Analyzer.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 101 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/12807/3
-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2:

The test failure in 2.x is

java.lang.AssertionError: Stmt didn't result in analysis error: select cast(2 as decimal(38, 37)) in (cast(2 as decimal(38, 1)))
	at org.junit.Assert.fail(Assert.java:88)
	at org.apache.impala.common.FrontendTestBase.AnalysisError(FrontendTestBase.java:415)
	at org.apache.impala.common.FrontendTestBase.AnalysisError(FrontendTestBase.java:395)
	at org.apache.impala.analysis.AnalyzeExprsTest.TestDecimalArithmetic(AnalyzeExprsTest.java:2498)

The newly added tests depend on setting decimal_v2=true which is false in 2.x by default. I add an AnalysisContext to turn on the decimal_v2 before these tests. See the difference between patch set 1 and patch set 2 about what I change.


-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 02:08:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 15:11:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12807/2/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/12807/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2473
PS2, Line 2473:     AnalysisError("select cast(2 as decimal(38, 37)) in (cast(2 as decimal(38, 1)))", decimal_v2_ctx,
line too long (101 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 02:11:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2476/ : 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/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 03:07:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2520/ : 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/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 22 Mar 2019 05:11:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12807 )

Change subject: IMPALA-7254: Inconsistent decimal behavior for IN/BETWEEN predicate
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3926/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/12807
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44f6ea45f765be7201630541354c72fda0a8a98d
Gerrit-Change-Number: 12807
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 13:29:22 +0000
Gerrit-HasComments: No