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/08 01:11:37 UTC
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Hello Jean-Daniel Cryans, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4984
to review the following change.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
KUDU-1599. Reject invalid type/encoding pairs on table creation
Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/client-test.cc
M src/kudu/master/catalog_manager.cc
3 files changed, 40 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/4984/1
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
KUDU-1599. Reject invalid type/encoding pairs on table creation
Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Reviewed-on: http://gerrit.cloudera.org:8080/4984
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/client-test.cc
M src/kudu/master/catalog_manager.cc
3 files changed, 39 insertions(+), 8 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4984/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
Line 3646: ASSERT_OK(schema_builder.Build(&schema));
> Interesting; why didn't we fail here instead?
I don't think the client should be the one responsible for checking the validity of type/encoding pairs, since eg then we'd need to update the client when the server starts to support RLE for int64, but maybe the client is talking to an old server which doesn't support it.
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4984/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS1, Line 3641: gscoped_ptr
> Nit: unique_ptr for new stuff.
Done
Line 3650: .num_replicas(3)
> Can be omitted, I think.
Done
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has uploaded a new patch set (#2).
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
KUDU-1599. Reject invalid type/encoding pairs on table creation
Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
---
M src/kudu/cfile/type_encodings.cc
M src/kudu/client/client-test.cc
M src/kudu/master/catalog_manager.cc
3 files changed, 39 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/4984/2
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4984/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
PS1, Line 3641: gscoped_ptr
Nit: unique_ptr for new stuff.
Line 3646: ASSERT_OK(schema_builder.Build(&schema));
Interesting; why didn't we fail here instead?
Line 3650: .num_replicas(3)
Can be omitted, I think.
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1599. Reject invalid type/encoding pairs on table creation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1599. Reject invalid type/encoding pairs on table creation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4984
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib508dbfbfd574c7c8ccc90ef8c3a41dcbfea058f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No