You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2016/11/03 11:12:21 UTC

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param 

David Ribeiro Alves has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4928

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................

Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

All the other methods in this class, of this genre, take one but
this one was taking the raw faststring buffer insted. Though it
did have the advantage of not forcing the caller to clear() the
buffer everytime it was weirdly out of place and might introduce
subtle bugs with new semantics for RowChangeListEncoder.

Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/tablet/memrowset.cc
3 files changed, 11 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/4928/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4928/10/src/kudu/common/row_changelist.cc
File src/kudu/common/row_changelist.cc:

Line 171:                                                RowChangeListEncoder* out) {
> probably should DCHECK that 'out' is uninitialized if that's the intention 
Done


http://gerrit.cloudera.org:8080/#/c/4928/10/src/kudu/common/row_changelist.h
File src/kudu/common/row_changelist.h:

PS10, Line 359:   // The buffer will be cleared before adding the result and will be empty if the projection
              :   // ends up producing an empty RowChangeList.
              :  
> this comment needs to be updated -- probably should mention whether 'out' s
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 13: Verified+1

unrelated flake RpcBench.BenchmarkCallsAsync

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4928

to look at the new patch set (#12).

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................

Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

All the other methods in this class, of this genre, take one but
this one was taking the raw faststring buffer instead. Though it
did have the advantage of not forcing the caller to clear() the
buffer everytime it was weirdly out of place and might introduce
subtle bugs with new semantics for RowChangeListEncoder.

Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/tablet/memrowset.cc
3 files changed, 13 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/4928/12
-- 
To view, visit http://gerrit.cloudera.org:8080/4928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 11: -Code-Review

oops missed todd's comments. reverting

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has uploaded a new patch set (#7).

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................

Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

All the other methods in this class, of this genre, take one but
this one was taking the raw faststring buffer instead. Though it
did have the advantage of not forcing the caller to clear() the
buffer everytime it was weirdly out of place and might introduce
subtle bugs with new semantics for RowChangeListEncoder.

Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/tablet/memrowset.cc
3 files changed, 11 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/4928/7
-- 
To view, visit http://gerrit.cloudera.org:8080/4928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4928/5//COMMIT_MSG
Commit Message:

PS5, Line 10: insted
nit: instead


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 5: Verified+1

Unrelated flake

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 11: Code-Review+2

again, keeping the +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

All the other methods in this class, of this genre, take one but
this one was taking the raw faststring buffer instead. Though it
did have the advantage of not forcing the caller to clear() the
buffer everytime it was weirdly out of place and might introduce
subtle bugs with new semantics for RowChangeListEncoder.

Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Reviewed-on: http://gerrit.cloudera.org:8080/4928
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/tablet/memrowset.cc
3 files changed, 13 insertions(+), 13 deletions(-)

Approvals:
  David Ribeiro Alves: Verified
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 8: Code-Review+2

just a rebase, keeping +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4928

to look at the new patch set (#10).

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................

Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

All the other methods in this class, of this genre, take one but
this one was taking the raw faststring buffer instead. Though it
did have the advantage of not forcing the caller to clear() the
buffer everytime it was weirdly out of place and might introduce
subtle bugs with new semantics for RowChangeListEncoder.

Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
---
M src/kudu/common/row_changelist.cc
M src/kudu/common/row_changelist.h
M src/kudu/tablet/memrowset.cc
3 files changed, 11 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/4928/10
-- 
To view, visit http://gerrit.cloudera.org:8080/4928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4928/10/src/kudu/common/row_changelist.cc
File src/kudu/common/row_changelist.cc:

Line 171:                                                RowChangeListEncoder* out) {
probably should DCHECK that 'out' is uninitialized if that's the intention here


http://gerrit.cloudera.org:8080/#/c/4928/10/src/kudu/common/row_changelist.h
File src/kudu/common/row_changelist.h:

PS10, Line 359:   // The buffer will be cleared before adding the result and will be empty if the projection
              :   // ends up producing an empty RowChangeList.
              :  
this comment needs to be updated -- probably should mention whether 'out' should already have its type set, or be uninitialized


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 12: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4928/5//COMMIT_MSG
Commit Message:

PS5, Line 10: insted
> nit: instead
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: Make RowChangeListDecoder::RemoveColumnIdsFromChangeList() take a RowChangeListEncoder as an out param
......................................................................


Patch Set 10: Code-Review+2

just a rebase keeping the +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6584b22662b58227a80eaebb7d89199d3ea2498a
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No