You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Marshall (Code Review)" <ge...@cloudera.org> on 2018/05/17 17:51:13 UTC
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10441
Change subject: IMPALA-6929: Support multi-column range partitions for Kudu
......................................................................
IMPALA-6929: Support multi-column range partitions for Kudu
Kudu allows specifying range partitions over multiple columns. Impala
already has support for doing this when the partitions are specified
with '=', but if the partitions are specified with '<' or '<=', the
parser would return an error.
This patch modifies the parser to allow for creating Kudu tables like:
create table kudu_test (a int, b int, primary key(a, b))
partition by range(a, b) (partition (0, 0) <= values < (1, 1));
and similary to alter partitions like:
alter table kudu_test add range partition (1, 1) <= values < (2, 2);
Testing:
- Modified functional_kudu.jointbl's schema so that we have a table
in functional with a multi-column range partition to test things
against.
- Added FE and E2E tests for CREATE and ALTER.
Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
6 files changed, 104 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/10441/1
--
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: newchange
Gerrit-Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
Gerrit-Change-Number: 10441
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
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 <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 17:35:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
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 <th...@cmu.edu>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 22:58:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10441 )
Change subject: IMPALA-6929: Support multi-column range partitions for Kudu
......................................................................
IMPALA-6929: Support multi-column range partitions for Kudu
Kudu allows specifying range partitions over multiple columns. Impala
already has support for doing this when the partitions are specified
with '=', but if the partitions are specified with '<' or '<=', the
parser would return an error.
This patch modifies the parser to allow for creating Kudu tables like:
create table kudu_test (a int, b int, primary key(a, b))
partition by range(a, b) (partition (0, 0) <= values < (1, 1));
and similary to alter partitions like:
alter table kudu_test add range partition (1, 1) <= values < (2, 2);
Testing:
- Modified functional_kudu.jointbl's schema so that we have a table
in functional with a multi-column range partition to test things
against.
- Added FE and E2E tests for CREATE and ALTER.
Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
Reviewed-on: http://gerrit.cloudera.org:8080/10441
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
6 files changed, 115 insertions(+), 33 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
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: merged
Gerrit-Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
Gerrit-Change-Number: 10441
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
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:
This change is small and low risk, so I think we can go ahead and merge it
--
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 <th...@cmu.edu>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jun 2018 20:35:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
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 2: Code-Review+2
--
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 <th...@cmu.edu>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 23:45:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Hello Vuk Ercegovac,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10441
to look at the new patch set (#2).
Change subject: IMPALA-6929: Support multi-column range partitions for Kudu
......................................................................
IMPALA-6929: Support multi-column range partitions for Kudu
Kudu allows specifying range partitions over multiple columns. Impala
already has support for doing this when the partitions are specified
with '=', but if the partitions are specified with '<' or '<=', the
parser would return an error.
This patch modifies the parser to allow for creating Kudu tables like:
create table kudu_test (a int, b int, primary key(a, b))
partition by range(a, b) (partition (0, 0) <= values < (1, 1));
and similary to alter partitions like:
alter table kudu_test add range partition (1, 1) <= values < (2, 2);
Testing:
- Modified functional_kudu.jointbl's schema so that we have a table
in functional with a multi-column range partition to test things
against.
- Added FE and E2E tests for CREATE and ALTER.
Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
6 files changed, 115 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/10441/2
--
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: newpatchset
Gerrit-Change-Id: I0141dd3344a4f22b186f513b7406f286668ef1e7
Gerrit-Change-Number: 10441
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 3: Code-Review+2
--
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: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jun 2018 20:35:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2644/
--
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: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jun 2018 20:35:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6929: Support multi-column range partitions for Kudu
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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 3: Verified+1
--
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: 3
Gerrit-Owner: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jun 2018 00:10:12 +0000
Gerrit-HasComments: No