Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12205 )
Change subject: KUDU-2645: tablet: Support deduplication of deleted rows in MergeIterator ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc File src/kudu/common/generic_iterators-test.cc: http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc@287 PS8, Line 287: ASSERT_EQ(1, expected_ints_deleted.erase(potential)); Nit: you can upgrade this to CHECK_EQ; the ContainsKey() check on L286 guarantees that it's true, so it's not really something being tested (i.e. something that warrants an assert). http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators-test.cc@369 PS8, Line 369: int is_deleted_idx = schema.find_first_is_deleted_virtual_column(); This can be pulled out of the loops. http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/12205/8/src/kudu/common/generic_iterators.cc@39 PS8, Line 39: #include "kudu/common/common.pb.h" Any idea what this inclusion is for? -- To view, visit http://gerrit.cloudera.org:8080/12205 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00614b3fa5c6b4e7b620bb78489e24c5ad44daee Gerrit-Change-Number: 12205 Gerrit-PatchSet: 8 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 06 Mar 2019 00:57:03 +0000 Gerrit-HasComments: Yes
