Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10441 )
Change subject: IMPALA-6929: Support multi-column range partitions for Kudu ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/10441/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java File fe/src/main/java/org/apache/impala/analysis/RangePartition.java: http://gerrit.cloudera.org:8080/#/c/10441/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java@46 PS1, Line 46: PARTITION (l_val1, ..., l_valn) <[=] VALUES : * - Single value (no range): > needs to updated? Done http://gerrit.cloudera.org:8080/#/c/10441/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java@81 PS1, Line 81: > nit: and ALTER Done http://gerrit.cloudera.org:8080/#/c/10441/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/10441/1/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@528 PS1, Line 528: '1' > why was this change needed? The point of this test case is that all of the exprs that target partition columns are constants. Previously, the only partition column in the table was 'test_id', so only the first expr in this select list needed to be constant, but my change to functional_schema_template.sql makes it so that 'test_name' is also a partition column, so the expr that is targeting 'test_name' needs to be a constant now too. http://gerrit.cloudera.org:8080/#/c/10441/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test File testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test: http://gerrit.cloudera.org:8080/#/c/10441/1/testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test@575 PS1, Line 575: ('a', 0) <= values < ('b', 1)) > just for my own info: what happens if the specified ranges overlap? what wi Overlapping ranges aren't allowed. Detecting this is a bit tricky, so we don't try and just pass the values off to Kudu, which returns an error. There was already a test case here for that in the single-range-column case, and I added one below for the multi-range-column case. -- To view, visit http://gerrit.cloudera.org:8080/10441 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7 Gerrit-Change-Number: 10441 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 08 Jun 2018 22:58:25 +0000 Gerrit-HasComments: Yes
