rdblue commented on a change in pull request #1325:
URL: https://github.com/apache/iceberg/pull/1325#discussion_r469452497
##########
File path: spark3/src/main/java/org/apache/iceberg/spark/Spark3Util.java
##########
@@ -178,8 +178,12 @@ private static void apply(UpdateSchema pendingUpdate,
TableChange.UpdateColumnPo
private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn
add) {
Type type = SparkSchemaUtil.convert(add.dataType());
- pendingUpdate.addColumn(parentName(add.fieldNames()),
leafName(add.fieldNames()), type, add.comment());
-
+ if (add.isNullable()) {
+ pendingUpdate.addColumn(parentName(add.fieldNames()),
leafName(add.fieldNames()), type, add.comment());
+ } else {
+ pendingUpdate.allowIncompatibleChanges()
Review comment:
I'm not sure that this should call `allowIncompatibleChanges()` because
adding a required column when there are no existing values will break reading
the new column in any table with data in it. The only time it is safe to add a
required column is if there is no data in the table.
What about throwing an exception here instead? I agree that the column
should not be optional if NOT NULL was specified.
Another alternative is to check whether the table has data and allow the
incompatible change if it doesn't have any rows.
----------------------------------------------------------------
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]
