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 2016/11/30 01:47:34 UTC

[kudu-CR] Remove GROUP VARINT encoding

Hello Dan Burkert,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Remove GROUP_VARINT encoding
......................................................................

Remove GROUP_VARINT encoding

This encoding was only ever implemented for the UINT32 type, and users
can no longer specify this type, since we only support signed ints.
Thus, there's no sense carrying around this baggage.

This patch removes the encoding as well as associated tests.

Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
---
M src/kudu/cfile/CMakeLists.txt
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/encoding-test.cc
D src/kudu/cfile/gvint_block.cc
D src/kudu/cfile/gvint_block.h
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/schema.h
M src/kudu/common/common.proto
10 files changed, 11 insertions(+), 626 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>

[kudu-CR] Remove GROUP VARINT encoding

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

Change subject: Remove GROUP_VARINT encoding
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] Remove GROUP VARINT encoding

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

Change subject: Remove GROUP_VARINT encoding
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No

[kudu-CR] Remove GROUP VARINT encoding

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

Change subject: Remove GROUP_VARINT encoding
......................................................................


Remove GROUP_VARINT encoding

This encoding was only ever implemented for the UINT32 type, and users
can no longer specify this type, since we only support signed ints.
Thus, there's no sense carrying around this baggage.

This patch removes the encoding as well as associated tests.

Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Reviewed-on: http://gerrit.cloudera.org:8080/5271
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/cfile/CMakeLists.txt
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/encoding-test.cc
D src/kudu/cfile/gvint_block.cc
D src/kudu/cfile/gvint_block.h
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/schema.h
M src/kudu/common/common.proto
M src/kudu/tablet/diskrowset-test.cc
11 files changed, 14 insertions(+), 629 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Remove GROUP VARINT encoding

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins,

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

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

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

Change subject: Remove GROUP_VARINT encoding
......................................................................

Remove GROUP_VARINT encoding

This encoding was only ever implemented for the UINT32 type, and users
can no longer specify this type, since we only support signed ints.
Thus, there's no sense carrying around this baggage.

This patch removes the encoding as well as associated tests.

Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
---
M src/kudu/cfile/CMakeLists.txt
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.h
M src/kudu/cfile/encoding-test.cc
D src/kudu/cfile/gvint_block.cc
D src/kudu/cfile/gvint_block.h
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/schema.h
M src/kudu/common/common.proto
M src/kudu/tablet/diskrowset-test.cc
11 files changed, 14 insertions(+), 629 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/5271/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5271
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0b0aeb5bfd8451a6e190ccb6e55e971da668b062
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot