Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17879 )
Change subject: WIP KUDU-2671: Follow up pruning patch. ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/17879/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17879/5//COMMIT_MSG@25 PS5, Line 25: dummy_value > This variable isn't actually meant to be set, it just allows one to call Ch Most likely I don't understand something here, but INT<NUM_OF_BITS>_MIN and INT<NUM_OF_BITS>_MAX look like a good option to be of 'no-boundary'/'boundary-is-not-set' semantics. Otherwise, could the semantics of std::optional<> be helpful there? https://en.cppreference.com/w/cpp/utility/optional http://gerrit.cloudera.org:8080/#/c/17879/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17879/6//COMMIT_MSG@19 PS6, Line 19: : The current tests don't include tables with non covering ranges. : They also don't include scans with equality predicates (only : greater or less than Do you plan to add a coverage for these as well in a follow-up patch or that's going to be a part of this patch as well? http://gerrit.cloudera.org:8080/#/c/17879/6/src/kudu/client/flex_partitioning_client-test.cc File src/kudu/client/flex_partitioning_client-test.cc: http://gerrit.cloudera.org:8080/#/c/17879/6/src/kudu/client/flex_partitioning_client-test.cc@250 PS6, Line 250: Should this be INT8_MAX instead? -- To view, visit http://gerrit.cloudera.org:8080/17879 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a1bf5344c0ef856072d3ed102714dce5ba21060 Gerrit-Change-Number: 17879 Gerrit-PatchSet: 5 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Jun 2022 18:26:28 +0000 Gerrit-HasComments: Yes
