Vuk Ercegovac 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 1:

(4 comments)

looks good, mainly nits and clarifying questions.

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: Multi-value:
            :  *   PARTITION VALUE = (val1, val2, ..., valn)
needs to updated?


http://gerrit.cloudera.org:8080/#/c/10441/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java@81
PS1, Line 81: CREATE
nit: and ALTER


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?


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 will 
happen to tuples that are inserted into the overlap? just wondering if analysis 
should attempt to ban such cases.



--
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: 1
Gerrit-Owner: Thomas Marshall <thomasmarsh...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 17:35:57 +0000
Gerrit-HasComments: Yes

Reply via email to