openinx commented on a change in pull request #2465:
URL: https://github.com/apache/iceberg/pull/2465#discussion_r618189003
##########
File path: core/src/main/java/org/apache/iceberg/SchemaUpdate.java
##########
@@ -408,11 +427,55 @@ private TableMetadata applyChangesToMapping(TableMetadata
metadata) {
private static Schema applyChanges(Schema schema, List<Integer> deletes,
Map<Integer, Types.NestedField> updates,
Multimap<Integer, Types.NestedField> adds,
- Multimap<Integer, Move> moves) {
+ Multimap<Integer, Move> moves,
+ Set<String> identifierNames) {
+ // validate existing identifier fields are not deleted
+ for (String name : identifierNames) {
+ Types.NestedField field = schema.findField(name);
Review comment:
If we have two columns `(a, b)` in the table, and the original
identifier fields is `a`. Then people want to delete the column `a` first and
then rename the column `b` to column `a` and also change the identifier fields
from old `a` to new `a`. As the identifier is a constraint for delete, so
people will need to call the API by :
```java
SchemaUpdate update = ...
update.deleteColumn("a").renameColumn("b", "a")
```
In this way, he will encounter an argument exception and try to call the
`setIdentifierFields`:
```java
update.setIdentifierFields("b")
.deleteColumn("a")
.renameColumn("b", "a")
.setIdentiferFields("a")
```
But this way, people will still encounter the exception. So maybe the final
solution will be:
```java
Step.1 update.setIdentifierFields(ImmutableSet.of()).commit();
Step.2 update.deleteColumn("a").renameColumn("b", "a").commit();
Step.3 update.setIdentifierFields("a").commit();
```
--
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]
