You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/07/07 00:14:39 UTC

[kudu-CR] tablet: encapsulate common iterator options

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

Change subject: tablet: encapsulate common iterator options
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10802/3/src/kudu/tablet/deltafile.cc
File src/kudu/tablet/deltafile.cc:

http://gerrit.cloudera.org:8080/#/c/10802/3/src/kudu/tablet/deltafile.cc@a369
PS3, Line 369: 
hmm, what happened to empty_schema? seems now we are letting it be an uninitialized pointer which seems scary


http://gerrit.cloudera.org:8080/#/c/10802/3/src/kudu/tablet/deltafile.cc@691
PS3, Line 691: tlipcon
nit: i usually use 'todd' since it's my apache id


http://gerrit.cloudera.org:8080/#/c/10802/3/src/kudu/tablet/rowset.h
File src/kudu/tablet/rowset.h:

http://gerrit.cloudera.org:8080/#/c/10802/3/src/kudu/tablet/rowset.h@67
PS3, Line 67:   OrderMode order;
seems there are lots of places where this struct is instantiated without actually setting all of the options. Should we have defaults for these fields?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7232d163436e69999bba75ed66756d2a86c5a959
Gerrit-Change-Number: 10802
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Sat, 07 Jul 2018 00:14:39 +0000
Gerrit-HasComments: Yes