nandorKollar commented on code in PR #14266:
URL: https://github.com/apache/iceberg/pull/14266#discussion_r2543678891


##########
core/src/main/java/org/apache/iceberg/SchemaUpdate.java:
##########
@@ -280,8 +301,17 @@ public UpdateSchema updateColumn(String name, 
Type.PrimitiveType newType) {
       return this;
     }
 
+    // If field is listed in source-ids, we need to flag it for promoting date 
-> timestamp.
+    List<PartitionField> partitionFields =
+        this.base != null
+            ? this.base.spec().getFieldsBySourceId(field.fieldId())
+            : Lists.newArrayList();
+
+    boolean isBucketPartitioned =
+        partitionFields.stream().anyMatch(pf -> 
pf.transform().toString().startsWith("bucket["));

Review Comment:
   I think blacklisting certain transformations here is very brittle. Is bucket 
transformation the only one which is problematic, which will produce different 
value after the promotion? I think currently it is. Is it remain the only one 
in the future? Probably not. How can we make this future-proof, so that 
transformations added later on don't break this? My fear is that it very easy 
to forget to change this line here. TBH, I'd simply not allow date promotion, 
if it is mentioned in partition field. What do you think? Does it sound too 
strict?



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to