Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11381 )
Change subject: KUDU-2566: fix two todo list ...................................................................... Patch Set 2: (2 comments) The formatting in this patch got kind of messed up; there's a lot of empty space between each new line of code. http://gerrit.cloudera.org:8080/#/c/11381/2/src/kudu/tablet/rowset_tree.h File src/kudu/tablet/rowset_tree.h: http://gerrit.cloudera.org:8080/#/c/11381/2/src/kudu/tablet/rowset_tree.h@88 PS2, Line 88: void FindRowSetsIntersectingInterval(const Slice &lower_bound, : const Slice &upper_bound, : std::vector<RowSet *> *rowsets) const; : void FindRowSetsIntersectingIntervalGE(const Slice& lower_bound, : std::vector<RowSet*> *rowsets) const; : void FindRowSetsIntersectingIntervalLT(const Slice& upper_bound, : std::vector<RowSet*> *rowsets) const; Rather than introducing new FindRowSetsIntersectingInterval... methods, could we rewrite the existing method like this: void FindRowSetsIntersectingInterval(std::vector<RowSet*>* rowsets, boost::optional<Slice> lower_bound, boost::optional<Slice> upper_bound) const; Then if you wanted to omit one of the bounds, it's a simple matter of passing boost::none in for it. The function could check that at least one bound was provided. http://gerrit.cloudera.org:8080/#/c/11381/2/src/kudu/util/interval_tree.h File src/kudu/util/interval_tree.h: http://gerrit.cloudera.org:8080/#/c/11381/2/src/kudu/util/interval_tree.h@140 PS2, Line 140: // Greater than or equal. : // The 'query' is the lower_bound. : template<class QueryPointType> : void FindIntersectingIntervalGE(const QueryPointType& query, : IntervalVector *results) const; : // Less than or equal. : // The 'query' is the upper_bound. : template<class QueryPointType> : void FindIntersectingIntervalLT(const QueryPointType &query, : IntervalVector *results) const; : Rather than introduce these new methods, would it be possible to modify interval_type and to allow infinity and negative infinity as end points? Then you could reuse FindIntersectingInterval for one-sided interval queries. -- To view, visit http://gerrit.cloudera.org:8080/11381 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e34b4169f93f3519f694c9bf86ca5295c318e79 Gerrit-Change-Number: 11381 Gerrit-PatchSet: 2 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Tue, 04 Sep 2018 18:41:20 +0000 Gerrit-HasComments: Yes
