You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "wangning (Code Review)" <ge...@cloudera.org> on 2020/12/24 08:24:42 UTC
[kudu-CR] KUDU-1644 use range partition info for pruning
wangning has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16903
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
KUDU-1644 use range partition info for pruning
When pruning in-list predicate values, range partition can also be taken
into consideration.
Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_service.cc
7 files changed, 543 insertions(+), 291 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/1
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 1
Gerrit-Owner: wangning <19...@gmail.com>
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16903
to look at the new patch set (#3).
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
KUDU-1644 use range partition info for pruning
When pruning in-list predicate values, range partition can also be taken
into consideration.
Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_service.cc
7 files changed, 661 insertions(+), 276 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/3
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "wangning (Code Review)" <ge...@cloudera.org>.
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@443
PS3, Line 443: // Verify the splitted values can merge into original set without overlapping.
> nit:original
Done
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@473
PS3, Line 473: // both hash and range aspects.
> nit:on both hash and range aspects? rather than either.
Done
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@694
PS3, Line 694: // on both hash and range aspects.
> See above comment.
Done
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Thu, 07 Jan 2021 07:42:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16903
to look at the new patch set (#2).
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
KUDU-1644 use range partition info for pruning
When pruning in-list predicate values, range partition can also be taken
into consideration.
Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_service.cc
7 files changed, 661 insertions(+), 276 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/2
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 2
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Mahesh Reddy, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16903
to look at the new patch set (#4).
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
KUDU-1644 use range partition info for pruning
When pruning in-list predicate values, range partition can also be taken
into consideration.
Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_service.cc
7 files changed, 662 insertions(+), 277 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16903/4
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
KUDU-1644 use range partition info for pruning
When pruning in-list predicate values, range partition can also be taken
into consideration.
Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Reviewed-on: http://gerrit.cloudera.org:8080/16903
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/scan_spec.cc
M src/kudu/common/scan_spec.h
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tserver/tablet_service.cc
7 files changed, 662 insertions(+), 277 deletions(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@443
PS3, Line 443: // Verify the splitted values can merge into originl set without overlapping.
nit:original
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@473
PS3, Line 473: // either hash and range aspects.
nit:on both hash and range aspects? rather than either.
http://gerrit.cloudera.org:8080/#/c/16903/3/src/kudu/common/scan_spec-test.cc@694
PS3, Line 694: // on either hash and range aspects.
See above comment.
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 22:31:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
Patch Set 4: Code-Review+2
Thanks for the patch!
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Thu, 07 Jan 2021 18:58:10 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "wangning (Code Review)" <ge...@cloudera.org>.
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG@9
PS1, Line 9: When pruning in-list predicate values, range partition can also be taken
> I can see this being particularly useful if range partitioning is defined p
We don't have heavy scenarios on range-schema based inList query. So I would do a small scale benchmark about this patch.
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@584
PS1, Line 584: AddInPredicate<int8_t>(&spec, "a", { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 });
: AddInPredicate<int8_t>(&spec, "b", { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 });
> So far, we have good test coverage for the cases where 'a' is pruned. Could
I just realize that range key can be differ from hash key. I adjusted some code for it. Thx for point out.
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@601
PS1, Line 601: b
> Should this be "b"?
Ah, yes, that explains why the previous patched result is so weird.
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 09:38:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1644 use range partition info for pruning
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16903 )
Change subject: KUDU-1644 use range partition info for pruning
......................................................................
Patch Set 1:
(4 comments)
Overall looks pretty good. Thanks for following up here!
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16903/1//COMMIT_MSG@9
PS1, Line 9: When pruning in-list predicate values, range partition can also be taken
I can see this being particularly useful if range partitioning is defined per hour or per day, and querying specific times. I'm curious, has your use case been able to leverage this optimization yet? If so, I'd love to hear more about your workloads.
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@82
PS1, Line 82: schema
nit: this is a "spec string" since it represents a scan spec, not a schema
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@584
PS1, Line 584: AddInPredicate<int8_t>(&spec, "a", { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 });
: AddInPredicate<int8_t>(&spec, "b", { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 });
So far, we have good test coverage for the cases where 'a' is pruned. Could we also add a case where both 'a' and 'b' are pruned, e.g. if 'a' is hash partitioned and 'b' is range partitioned? Similar to TestMultiHashKeyOneColumnInListHashPruning
http://gerrit.cloudera.org:8080/#/c/16903/1/src/kudu/common/scan_spec-test.cc@601
PS1, Line 601: a
Should this be "b"?
--
To view, visit http://gerrit.cloudera.org:8080/16903
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f14f543cffd44f090026f344c9b06af13ea2e10
Gerrit-Change-Number: 16903
Gerrit-PatchSet: 1
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 06 Jan 2021 00:48:15 +0000
Gerrit-HasComments: Yes