Ashwani Raina has posted comments on this change. (
http://gerrit.cloudera.org:8080/20690 )
Change subject: [cfile] clean up on IndexBlock{Builder,Iterator,Reader}
......................................................................
Patch Set 7:
(3 comments)
Overall looks good to me. Just a few minor comments.
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc
File src/kudu/cfile/index-test.cc:
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc@348
PS7, Line 348: TEST(TestIndexBlock, EmptyBlock) {
nit: Add a one liner about what this test does
Same goes for line number 378
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index-test.cc@404
PS7, Line 404: 2048
How is this different from 8 keys iterations above? Testing boundary condition?
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index_block.cc
File src/kudu/cfile/index_block.cc:
http://gerrit.cloudera.org:8080/#/c/20690/7/src/kudu/cfile/index_block.cc@268
PS7, Line 268: if (PREDICT_FALSE(data_.data() + offset_in_block >=
key_offsets_)) {
: return Status::Corruption(Substitute(
: "$0: invalid block offset at index $1", offset_in_block,
idx_in_block));
: }
Is it possible to cover this and other uncovered error scenarios in the unit
tests?
--
To view, visit http://gerrit.cloudera.org:8080/20690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If83dd132b577a481a2ddaa09e2657639f8b92c7d
Gerrit-Change-Number: 20690
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 15 Nov 2023 15:22:02 +0000
Gerrit-HasComments: Yes
