Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13130 )
Change subject: [backup] Add more metadata fields ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13130/1/java/kudu-backup/src/main/protobuf/backup.proto File java/kudu-backup/src/main/protobuf/backup.proto: http://gerrit.cloudera.org:8080/#/c/13130/1/java/kudu-backup/src/main/protobuf/backup.proto@120 PS1, Line 120: // A map of column name to internal column id. : // This is useful for detecting dropped and added columns. > Can you doc why column IDs are here and not in ColumnMetadataPB? You could > always build a map at runtime after loading the protobuf. I kept them separate because they are for separate things. Once is primarily for recreating the columns, while the other is primarily for validation. If someone were hand crafting a json file for the sake of creating a table, they don't need the ids. > Also, should we track all column IDs that ever existed? Or the max column ID? > Would either be useful to backup/restore? I don't have a reason for this yet. We can add it if we do. http://gerrit.cloudera.org:8080/#/c/13130/1/java/kudu-client/src/main/java/org/apache/kudu/Schema.java File java/kudu-client/src/main/java/org/apache/kudu/Schema.java: PS1: > Do you remember off-hand when the master supplies a schema with column IDs I am not positive off-hand, but I was under the impression Schemas from the server always have Ids. Schemas constructed by users client side don't. http://gerrit.cloudera.org:8080/#/c/13130/1/java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java File java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java: PS1: > Hmm, why isn't this in src/test/java? It's used in application code for non unit testing reasons like the DistributedDataGenerator. -- To view, visit http://gerrit.cloudera.org:8080/13130 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42458f598a523596acb9f18558e6f518719a969b Gerrit-Change-Number: 13130 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Comment-Date: Fri, 26 Apr 2019 20:59:21 +0000 Gerrit-HasComments: Yes