Andrew Wong has posted comments on this change.

Change subject: Inlined dispatch for predicate evaluation
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4164/7//COMMIT_MSG
Commit Message:

PS7, Line 25: 1M unique strings 
> is this "dictionary encoded" column actually just falling back to 'plain' m
Yes, the main beneficiary of this patch wrt EvaluateCell would be any encoding 
that has makes many calls to EvaluateCell, which is only 'plain' encoding at 
the moment. I'll elaborate a bit more here.


http://gerrit.cloudera.org:8080/#/c/4164/7/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:

Line 312:   CHECK_NOTNULL(sel);
> think this could be a DCHECK
Done. My thinking before was that CHECK should be used when the false case 
would break the code (eg here it would give a nullptr exception), but since we 
know this shouldn't be null based on our own code, DCHECK makes sense 
(particularly since this is on the hot path).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iccfac9bc899362b442337050795b5ca8c4101268
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to