rdblue commented on a change in pull request #2465:
URL: https://github.com/apache/iceberg/pull/2465#discussion_r612814134
##########
File path: api/src/main/java/org/apache/iceberg/Schema.java
##########
@@ -46,34 +48,53 @@
*/
public class Schema implements Serializable {
private static final Joiner NEWLINE = Joiner.on('\n');
+ private static final Joiner COMMA = Joiner.on(',');
private static final String ALL_COLUMNS = "*";
private static final int DEFAULT_SCHEMA_ID = 0;
private final StructType struct;
private final int schemaId;
+ private final int[] rowIdentifiers;
Review comment:
I think we need to fix the term used here. There isn't more than one
identifier, so it isn't quite right to call this "identifiers". We also want to
make sure that it is a bit more clear just what these are, so I think we should
clearly call out that what is returned are field IDs. That's why I originally
suggested `identifier-field-ids` for the JSON field and `identifierFieldIds`
for the `Schema` method.
I think it would be fine to use the "row" prefix, but I don't think that it
adds much value. I typically opt to go with the simpler option so I prefer
`identifierFieldIds`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
