Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3724: Support Kudu non-covering range partitions ......................................................................
Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/4856/5/fe/src/main/java/org/apache/impala/analysis/DistributeParam.java File fe/src/main/java/org/apache/impala/analysis/DistributeParam.java: Line 137: for (String colName: colNames_) pkColDefs.add(pkColumnDefByName_.get(colName)); > is pkColumnDefByName_.get(colName) guaranteed to be non-NULL? Yes, we've already checked that in analyze() before the call to this function. http://gerrit.cloudera.org:8080/#/c/4856/5/fe/src/main/java/org/apache/impala/analysis/RangePartition.java File fe/src/main/java/org/apache/impala/analysis/RangePartition.java: Line 150: LiteralExpr literal = LiteralExpr.create(value, analyzer.getQueryCtx()); > Can't we move this after the implicit casting? That way we'll only have one I have one example where this doesn't seem to work. e.g. PARTITION VALUES < factorial(4), where the partition column is type INT. Because factorial returns type BIGINT the check in L162 throws an error. However, by doing the folding first the checks pass as the return value can be implicitly casted to INT without precision loss. Thoughts? Line 169: Expr castedLiteral = literal.uncheckedCastTo(colType); > castLiteral Done http://gerrit.cloudera.org:8080/#/c/4856/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: Line 1859: AnalyzesOk("create table tab (a int primary key) distribute by range (a) " + > // Test implicit casting/folding of partition values. Done http://gerrit.cloudera.org:8080/#/c/4856/5/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test: Line 82: create table tab (a int primary key) distributed by range (a) (partition value = false) > add comment that this is testing implicit casting + folding Done -- To view, visit http://gerrit.cloudera.org:8080/4856 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6799c01a37003f0f4c068d911a13e3f060110a06 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
