Sameer Abhyankar has posted comments on this change.

Change subject: KUDU-1363: Add in-list predicates for extracting a set of 
equalities.
......................................................................


Patch Set 7:

(4 comments)

Hi Adar - The latest PS should have the additional changes you identified.

        The main reason for not using unordered_set was because the collection 
would still be storing pointers instead of actual values and I did not think I 
could get an O(1) lookup when i am searching for a pointer-to-a-value in a 
set-of-pointers. PredicateData doesnt know about the the type of the value and 
defers the comparison to TypeInfo. As a result, PredicateData only holds void* 
or a collection(vector) of void*.

        The other (not so critical) reason I went with a vector vs an 
unordered_set was because I do care about the order of the list (which is 
sorted/deduped) when I push Upper/Lower bound key predicates in key_util. I 
suppose, I could sort and pick the min/max values from the collection in 
key_util itself instead of using a vector. However, using a vector just made it 
convenient.

http://gerrit.cloudera.org:8080/#/c/2986/6/src/kudu/client/scan_predicate-internal.h
File src/kudu/client/scan_predicate-internal.h:

Line 27: #include "kudu/util/memory/arena.h"
> Don't need this anymore.
Done


http://gerrit.cloudera.org:8080/#/c/2986/6/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:

Line 519: TEST_F(TestColumnPredicate, TestInList) {
> Nit: separate TestInList from the comma after TestColumnPredicate with a sp
Done


http://gerrit.cloudera.org:8080/#/c/2986/6/src/kudu/common/types.h
File src/kudu/common/types.h:

Line 32: #include "kudu/util/slice.h"
> Is this still necessary?
Done


http://gerrit.cloudera.org:8080/#/c/2986/6/src/kudu/common/wire_protocol-test.cc
File src/kudu/common/wire_protocol-test.cc:

Line 27: #include "kudu/util/status.h"
> Don't need this?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/2986
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I986cb13097f1cf4af2b752f58c4c38f412a6a598
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sameer Abhyankar <abhyan...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sameer Abhyankar <abhyan...@gmail.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to