rdblue commented on a change in pull request #2465:
URL: https://github.com/apache/iceberg/pull/2465#discussion_r617736448
##########
File path: core/src/main/java/org/apache/iceberg/SchemaUpdate.java
##########
@@ -317,6 +333,38 @@ public UpdateSchema unionByNameWith(Schema newSchema) {
return this;
}
+ @Override
+ public UpdateSchema addIdentifierField(String name) {
+ Types.NestedField field = schema.findField(name);
+ if (field == null) {
+ field = adds.get(TABLE_ROOT_ID).stream()
+ .filter(f -> f.name().equals(name))
+ .findAny().orElse(null);
Review comment:
@openinx, that's what I was thinking at first as well, but I don't think
that we need a version with parent now.
The reason is that once a column is added, it is uniquely identified by a
dotted name: `a.b` might be a struct named `"a` with a field named `b` or might
be a field named `a.b`, but it cannot be both. We guarantee this so that we
never have ambiguous names.
Because we know that the string passed in here will always identify just one
field, we don't need to handle the ambiguous case by adding an explicit parent
like we do with `addColumn`.
I think that means we can have a much nicer API:
* `setIdentifierFields(String...)` and
`setIdentifierFields(Collection<String>)`
* `addIdentifierField(String)`
--
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]
