Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12890 )
Change subject: java: ColumnSchema supports storing column comment ...................................................................... Patch Set 1: (7 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() { Maybe we could default to empty string instead of null. It might make users code a bit easier. http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@244 PS1, Line 244: if (comment != null) { If we default to "" this check wont be needed. 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@296 PS1, Line 296: pb.setSchema(ProtobufHelper.schemaToPb(lowerBound.getSchema(), SchemaPBFlags.SCHEMA_PB_WITHOUT_COMMENT)); Why can't the schema have a comment in this case? http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@355 PS1, Line 355: pb.setSchema(ProtobufHelper.schemaToPb(lowerBound.getSchema(), SchemaPBFlags.SCHEMA_PB_WITHOUT_COMMENT)); Why can't the schema have a comment in this case? 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) { Since setting a null comment isn't allowed, it makes more sense to default to empty string in the other places I mentioned. 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)); Why can't the schema have a comment in this case? Is this just to save space? http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java: http://gerrit.cloudera.org:8080/#/c/12890/1/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@150 PS1, Line 150: String comment = pb.hasComment() ? pb.getComment() : null; default empty string "". -- 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: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 29 Mar 2019 13:01:49 +0000 Gerrit-HasComments: Yes
