helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12890 )
Change subject: java/c++: ColumnSchema supports storing column comment ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/12890/2/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/2/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@401 PS2, Line 401: * Set the comment for this column. > What does empty string mean? Should it be allowed? Done http://gerrit.cloudera.org:8080/#/c/12890/2/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/2/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@365 PS2, Line 365: * Change the comment for the column. > Should doc what effect empty string has (it's for deleting an existing comm Done http://gerrit.cloudera.org:8080/#/c/12890/2/src/kudu/common/wire_protocol.cc File src/kudu/common/wire_protocol.cc: http://gerrit.cloudera.org:8080/#/c/12890/2/src/kudu/common/wire_protocol.cc@309 PS2, Line 309: attributes, type_attributes, std::move(comment)); > Now that empty string means "no comment", I don't think we need boost::opti Yeah, the boost::optional of comment_ in class ColumnSchema can be deleted. But in class KuduColumnSpec::Data & struct ColumnSchemaDelta they can't, because both of them need to represent three states while altering a table: 1) "user didn't ask us to modify the comment" 2) "user asked us to modify the comment" 3) "user asked us to delete the comment" ---------------------------------------------- In addition, c++ client uses the same class/struct to store the changes for creating and altering table, but java client does not. -- 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: 2 Gerrit-Owner: helifu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Wed, 03 Apr 2019 03:46:34 +0000 Gerrit-HasComments: Yes
