You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yongjun Zhang (Code Review)" <ge...@cloudera.org> on 2018/09/14 07:58:08 UTC

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent. 

Yongjun Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11443


Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it for Decimal V2 mode. V1 behavior
is left as is since the V1 mode will be dropped soon per IMPALA-4924.

Testing:
Added unit tests to cover the above mentioned two cases for both Decimal V1 and V2 mode,
while the two cases are fixed in V2 mode, they remain different in V1 mode.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 75 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9:

> (2 comments)

Thanks for the review Vuk. These two comments are addressed in rev9. I used "this case" instead of "these cases" since it's only one case there. Hope it works for you.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 16:37:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 6:

Thanks a lot for the review Vuk, just updated new rev to address your comments.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 21:54:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 20:41:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 8
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 01:31:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3162/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Sat, 15 Sep 2018 04:03:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 4
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 20 Sep 2018 21:06:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11443/1/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/11443/1/fe/src/main/java/org/apache/impala/analysis/Expr.java@448
PS1, Line 448: equals("mod")
this seems fairly special cased. I'm wondering if the functionCall expr is the place to do this... it seems to have special cases there for v2 type handling.


http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@515
PS1, Line 515: ---- QUERY
pls add unit tests as well to AnalyzeExprsTest.java. if '%' and 'mod' are equiv, then it should not be hard to extend all tests against the '%' arithmetic expr to also confirm the same behavior with 'mod'.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Sep 2018 18:08:16 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 3:

Uploaded a quick v3 to prove concept, it seems to be a much better solution. The reason I'm looking for new solution is, the casting in the previous solution can fail for certain cases. That's the hole I was referring to. I will craft coding and probably add more tests later. One side note, this will change the behavior of "mod" for certain cases, thus may appear incompatible. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:14:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1100
PS5, Line 1100:     StatementBase stmt = parse(queryCtx.client_request.stmt, queryCtx.client_request.query_options);
line too long (100 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 19:10:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2:

> (2 comments)

Very good suggestion Vuk, I tried it out, and added some more tests and uncovered a hole in the solution. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:20:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 6:

(2 comments)

Looks good to me, just had a couple of suggestions

http://gerrit.cloudera.org:8080/#/c/11443/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/11443/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@87
PS6, Line 87:   public static Expr createExpr(FunctionName fnName, FunctionParams params,
There are only two callsites for createExpr() so I think we should just update the other to pass through TQueryOptions too rather than making it nullable.

It would be easy for future coders to introduce an bug otherwise where they forget to pass through the query options and we don't apply the rewrite. That would be hard to catch.


http://gerrit.cloudera.org:8080/#/c/11443/6/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11443/6/fe/src/main/java/org/apache/impala/service/Frontend.java@1035
PS6, Line 1035:   public StatementBase parse(String stmt) throws AnalysisException {
Instead of threading null all the way through, can we just pass in a dummy TQueryOptions object. Best to keep test code paths close to the real ones where possible.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 23:28:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Sep 2018 07:59:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently in DECIMAL V2 mode, typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is
DECIMAL(4,1), while both are expected to be DECIMAL(2,1). This jira fixes V2 mode by
replacing "mod" with "%" at parser stage thus they share the same code path afterwards.

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
12 files changed, 126 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/8
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 8
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@84
PS5, Line 84:    * Expr is not necessarily a FunctionCallExpr (example: DECODE())
state that options can be null.


http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@108
PS5, Line 108: // IMPALA-6202: mod and % should be equivalent.
remove. git blame should be enough to see why we make them equiv.


http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@109
PS5, Line 109: When it's DECIMAL V2 mdoe, replace mod function with % operator, so they execute
             :     // the same code path.
simplify: '%' and 'mod' are equivalent when using DECIMAL V2.


http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@114
PS5, Line 114: State
Argument? (plist is a tmp that's derived from the argument).


http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/11443/5/fe/src/main/java/org/apache/impala/service/Frontend.java@1044
PS5, Line 1044:        parser.setQueryOptions(options);
make this one line


http://gerrit.cloudera.org:8080/#/c/11443/5/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/11443/5/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@524
PS5, Line 524: # IMPALA-6202 - mod and % s
I'd get rid of this line here, L532 and L540


http://gerrit.cloudera.org:8080/#/c/11443/5/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@533
PS5, Line 533: # Leave Decimal V1 mode as is.
keep this here to point out the difference for v1.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 20:17:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3160/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Sep 2018 11:40:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 4
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 20 Sep 2018 20:39:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 19:11:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it by making mod and % equivalent.
The solution is to replace "mod" with "%".

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
5 files changed, 91 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/4
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 4
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 8
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 02:15:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:45:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently in DECIMAL V2 mode, typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is
DECIMAL(4,1), while both are expected to be DECIMAL(2,1). This jira fixes V2 mode by
replacing "mod" with "%" at parser stage thus they share the same code path afterwards.

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
8 files changed, 120 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 20:10:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:02:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:14:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Wed, 26 Sep 2018 01:47:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 19:34:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:44:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Sat, 15 Sep 2018 00:30:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it by makeing mod and % equivalent.
The solution is to replace "mod" with "%".

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 79 insertions(+), 15 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 3
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 22:50:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9:

> (7 comments)

Thanks for the review Vuk, all the comments here are addressed in rev9.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 16:38:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/11443/1/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@516
PS1, Line 516: # IMPALA-6202 - mod and % are not equivalent.
rather than talk about how the old code used to work, it'd be better to word this in the way the code should work: "MOD and % should be equivalent". The current wording could be interpreted to mean that they are currently not equivalent.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Sep 2018 17:41:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2:

I'm thinking about a different approach now, say, we can probably simply convert a "mod" to "%" when doing the parsing, this way, all code used by "%" would be shared, and thus making them really equivalent. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 14 Sep 2018 23:37:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 23:00:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently in DECIMAL V2 mode, typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is
DECIMAL(4,1), while both are expected to be DECIMAL(2,1). This jira fixes V2 mode by
replacing "mod" with "%" at parser stage thus they share the same code path afterwards.

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
12 files changed, 126 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/9
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 16:19:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 10:

Hi Tim, Vuk and Dan, thanks a lot for the help with this jira. 

Best.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 10
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 22:05:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently in DECIMAL V2 mode, typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is
DECIMAL(4,1), while both are expected to be DECIMAL(2,1). This jira fixes V2 mode by
replacing "mod" with "%" at parser stage thus they share the same code path afterwards.

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
8 files changed, 125 insertions(+), 30 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/11443/5
-- 
To view, visit http://gerrit.cloudera.org:8080/11443
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 5
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 4
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 21 Sep 2018 00:18:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

Posted by "Yongjun Zhang (Code Review)" <ge...@cloudera.org>.
Yongjun Zhang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11443 )

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................

IMPALA-6202. mod and % are not equivalent.

Currently typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is DECIMAL(4,1), it's
expected that both to be DECIMAL(2,1). This jira fixes it for Decimal V2 mode. V1 behavior
is left as is since the V1 mode will be dropped soon per IMPALA-4924.

Testing:
Added unit tests to cover the above mentioned two cases for both Decimal V1 and V2 mode,
while the two cases are fixed in V2 mode, they remain different in V1 mode.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/TypesUtil.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
3 files changed, 72 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................

IMPALA-6202. mod and % should be equivalent.

Currently in DECIMAL V2 mode, typeof(9.9 % 3) is DECIMAL(2,1) and typeof(mod(9.9, 3)) is
DECIMAL(4,1), while both are expected to be DECIMAL(2,1). This jira fixes V2 mode by
replacing "mod" with "%" at parser stage thus they share the same code path afterwards.

Testing:
Added unit tests and done real cluster testing.

Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Reviewed-on: http://gerrit.cloudera.org:8080/11443
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
12 files changed, 126 insertions(+), 32 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 10
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 2:

Hi Dan and Vuk, thanks a lot for your review comments. I just uploaded a new rev to address some test failures. I will address your comments in next rev. Thanks.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 2
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Fri, 14 Sep 2018 20:11:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % are not equivalent.

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

Change subject: IMPALA-6202. mod and % are not equivalent.
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 1
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Sep 2018 08:33:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 6
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Tue, 25 Sep 2018 21:56:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 16:49:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 8
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 05:23:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6202. mod and % should be equivalent.

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

Change subject: IMPALA-6202. mod and % should be equivalent.
......................................................................


Patch Set 9:

> (2 comments)
 > 
 > Looks good to me, just had a couple of suggestions

Thanks for the review Tim, all your comments are addressed in newer rev.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0067da04083859ffbf662a29007154461bea2ba
Gerrit-Change-Number: 11443
Gerrit-PatchSet: 9
Gerrit-Owner: Yongjun Zhang <yj...@apache.org>
Gerrit-Reviewer: Dan Hecht <dh...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Yongjun Zhang <yj...@apache.org>
Gerrit-Comment-Date: Thu, 27 Sep 2018 16:35:36 +0000
Gerrit-HasComments: No