helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12890 )
Change subject: java: ColumnSchema supports storing column comment ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java: http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@213 PS1, Line 213: public String getComment() { > Apologies I didn't review that earlier patch. Maybe we need a consensus view of comment first: 1) Is "empty comment" valid? 2) Whether deleting comment is supported? 3) Should we add a new interface to replace "empty comment" for deleting comment? http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java: http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@368 PS1, Line 368: if (comment == null) { > Well, we do we want to support _deleting_ a column, right? Wouldn't we mode I am sorry i cannot catch your meaning here :( http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@283 PS1, Line 283: requestBuilder.setSchema(ProtobufHelper.schemaToPb(schema, SchemaPBFlags.SCHEMA_PB_WITHOUT_COMMENT)); > This makes sense. Even if a client sends a comment, we should probably stri I am afraid it is not a good idea for servers to focus on stripping comments to avoid WAL bloating, and more importantly that will bring a noticeable effect on WAL performance. -- To view, visit http://gerrit.cloudera.org:8080/12890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4cdca4101898062cfe154c15ca40c5943d0e343c Gerrit-Change-Number: 12890 Gerrit-PatchSet: 1 Gerrit-Owner: helifu <hzhel...@corp.netease.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <hzhel...@corp.netease.com> Gerrit-Comment-Date: Sun, 31 Mar 2019 08:07:22 +0000 Gerrit-HasComments: Yes
