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 2022/07/12 23:55:32 UTC
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18721
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
KUDU-2671: Extra test coverage for KuduScanTokens.
This patch adds extra test coverage for scans using
KuduScanTokens. Specifically, the test case is on a
table with custom hash schemas and non covering ranges.
Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
---
M src/kudu/client/client.h
M src/kudu/client/scan_token-test.cc
2 files changed, 153 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/18721/1
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18721 )
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 02:09:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18721 )
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
Patch Set 2: Verified+1
unrelated test failure: ReplicatedAlterTableTest.AlterReplicationFactorWhileWriting
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 02:09:13 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18721 )
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
KUDU-2671: Extra test coverage for KuduScanTokens.
This patch adds extra test coverage for scans using
KuduScanTokens. Specifically, the test case is on a
table with custom hash schemas and non covering ranges.
Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Reviewed-on: http://gerrit.cloudera.org:8080/18721
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/client/client.h
M src/kudu/client/scan_token-test.cc
2 files changed, 208 insertions(+), 10 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18721 )
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@748
PS1, Line 748: unique_ptr<KuduPartialRow> lower_bound(schema.NewRow());
: unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
nit: instead of defining these and ten calling 'reset' every time upon reuse in a few scopes below, maybe move these into the corresponding scopes?
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@761
PS1, Line 761:
nit: wrong indent
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@772
PS1, Line 772:
nit: wrong indent
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@773
PS1, Line 773: range_partition->add_hash_partitions({ "col"}, 2);
nit: missing space?
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@886
PS1, Line 886: }
Do you mind adding sub-scenarios for the following:
* predicate defining the exactly non-covered range, i.e. 100 <= x < 200
* predicate defining a range that intersects with both covered ranges and the non-covered range, i.e. 50 <= x < 250
* predicate defining a range that covers all the ranges and also the ranges at the left and the right, i.e. -50 <= x < 350
?
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 00:17:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18721 )
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc
File src/kudu/client/scan_token-test.cc:
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@748
PS1, Line 748: table_creator->table_name("table");
: table_creator->num_replicas(1);
> nit: instead of defining these and ten calling 'reset' every time upon reus
Done
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@761
PS1, Line 761:
> nit: wrong indent
Done
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@772
PS1, Line 772:
> nit: wrong indent
Done
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@773
PS1, Line 773: }
> nit: missing space?
Done
http://gerrit.cloudera.org:8080/#/c/18721/1/src/kudu/client/scan_token-test.cc@886
PS1, Line 886: "col", KuduPredicate::EQUAL, KuduValue::FromInt(242)));
> Do you mind adding sub-scenarios for the following:
Done
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Jul 2022 01:40:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2671: Extra test coverage for KuduScanTokens.
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18721
to look at the new patch set (#2).
Change subject: KUDU-2671: Extra test coverage for KuduScanTokens.
......................................................................
KUDU-2671: Extra test coverage for KuduScanTokens.
This patch adds extra test coverage for scans using
KuduScanTokens. Specifically, the test case is on a
table with custom hash schemas and non covering ranges.
Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
---
M src/kudu/client/client.h
M src/kudu/client/scan_token-test.cc
2 files changed, 208 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/18721/2
--
To view, visit http://gerrit.cloudera.org:8080/18721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0dec5c713940c1f8d66416c6a287b93e250d90e6
Gerrit-Change-Number: 18721
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>