Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13666 )

Change subject: KUDU-2866. Optimize CFileSet::Iterator::FinishBatch
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13666/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13666/2//COMMIT_MSG@16
PS2, Line 16: will only be prepared once
> Most likely I'm missing something, but from breefly looking into the implem
Currently I think we are limited to a single predicate per column, since 
ScanSpec::predicates_ is a map from column name to single ColumnPredicate 
instance. In the case where we have multiple predicates on the same column, we 
currently always merge them.

Then in MaterializeBlock(), we will only materialize each column once, because 
we separate the columns into those with predicates and those without.

Test coverage so far seems to agree. But I might believe that we have a lack of 
coverage and I missed something. Any idea of how we might produce the situation 
you're worried about?


http://gerrit.cloudera.org:8080/#/c/13666/2/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:

http://gerrit.cloudera.org:8080/#/c/13666/2/src/kudu/tablet/cfile_set.cc@470
PS2, Line 470:   prepared_iters_.clear();
> nit: does it make sense to add
I don't think we'd gain much from that, since 'clear' doesn't actually shrink 
the backing array. So, if on the first batch, we do end up using the full list 
of iters here, it'll keep that same "shape" across iterations.

I could see doing that in CFileSet::Iterator::Init, though. I'll add it there.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I997fe832fdfa8d92fbbcb5d7c5bd4141e485b4f8
Gerrit-Change-Number: 13666
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Tue, 18 Jun 2019 22:53:52 +0000
Gerrit-HasComments: Yes

Reply via email to