You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mahesh Reddy (Code Review)" <ge...@cloudera.org> on 2021/09/10 06:13:16 UTC

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17841


Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................

KUDU-2671: Pruning compatible with unbounded ranges.

This patch fixes an edge case where the upper bound is unbounded.
An existing test case is updated to include ranges with custom
hash schemas where both the lower and upper bound are unbounded.

Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
4 files changed, 309 insertions(+), 286 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/17841/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17841 )

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................

KUDU-2671: Pruning compatible with unbounded ranges.

This patch fixes an edge case where the absolute upper range
bound is unbounded. An existing test case is updated making both
the absoute lower and absolute upper range bounds unbounded.

Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Reviewed-on: http://gerrit.cloudera.org:8080/17841
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
4 files changed, 308 insertions(+), 286 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17841 )

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG@9
PS1, Line 9: range
> prefer to keep unbounded, more consistent with terminology used elsewhere.
OK, that makes sense to me.



-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 04:29:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17841

to look at the new patch set (#2).

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................

KUDU-2671: Pruning compatible with unbounded ranges.

This patch fixes an edge case where the absolute upper range
bound is unbounded. An existing test case is updated making both
the absoute lower and absolute upper range bounds unbounded.

Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
---
M src/kudu/client/flex_partitioning_client-test.cc
M src/kudu/common/key_util.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/partition_pruner.cc
4 files changed, 308 insertions(+), 286 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/17841/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17841 )

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG@9
PS1, Line 9: range
> nit: absent ?
prefer to keep unbounded, more consistent with terminology used elsewhere.


http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG@11
PS1, Line 11: unded.
> nit: absent ?
same as above


http://gerrit.cloudera.org:8080/#/c/17841/1/src/kudu/common/partition_pruner.cc
File src/kudu/common/partition_pruner.cc:

http://gerrit.cloudera.org:8080/#/c/17841/1/src/kudu/common/partition_pruner.cc@459
PS1, Line 459:         if (range.upper.empty() || scan_range_lower_bound < range.upper) {
> Good catch!  It seems we might benefit from introducing a special class wra
Ack



-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 04:10:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17841 )

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................


Patch Set 2: Verified+1

unrelated test failures:
  * org.apache.kudu.client.TestSecurity
  * MasterReplicationAndRpcSizeLimitTest.TabletReports


-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Sat, 11 Sep 2021 05:17:48 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2671: Pruning compatible with unbounded ranges.

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17841 )

Change subject: KUDU-2671: Pruning compatible with unbounded ranges.
......................................................................


Patch Set 1: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG@9
PS1, Line 9: unbounded
nit: absent ?


http://gerrit.cloudera.org:8080/#/c/17841/1//COMMIT_MSG@11
PS1, Line 11: unbounded
nit: absent ?


http://gerrit.cloudera.org:8080/#/c/17841/1/src/kudu/common/partition_pruner.cc
File src/kudu/common/partition_pruner.cc:

http://gerrit.cloudera.org:8080/#/c/17841/1/src/kudu/common/partition_pruner.cc@459
PS1, Line 459:         if (range.upper.empty() || scan_range_lower_bound < range.upper) {
Good catch!  It seems we might benefit from introducing a special class wrapper for a range's boundary and implement corresponding comparison operators to avoid mistakes like this in the future.



-- 
To view, visit http://gerrit.cloudera.org:8080/17841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic11bc7ceac7f3863c9d7e3440813f1faa3860b8e
Gerrit-Change-Number: 17841
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 11 Sep 2021 01:37:47 +0000
Gerrit-HasComments: Yes