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 <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Yongjun Zhang <[email protected]> Gerrit-Comment-Date: Tue, 25 Sep 2018 20:17:57 +0000 Gerrit-HasComments: Yes
