amogh-jahagirdar commented on code in PR #10991:
URL: https://github.com/apache/iceberg/pull/10991#discussion_r1727855740
##########
core/src/main/java/org/apache/iceberg/SchemaUpdate.java:
##########
@@ -285,7 +285,7 @@ public UpdateSchema updateColumn(String name,
Type.PrimitiveType newType) {
}
Preconditions.checkArgument(
- TypeUtil.isPromotionAllowed(field.type(), newType),
+ TypeUtil.isPromotionAllowed(field, newType, base.specs(),
base.formatVersion()),
Review Comment:
I need to add API level tests, at the moment only have a simple test via
Spark
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAlterTablePartitionFields.java:
##########
@@ -102,6 +112,41 @@ public void testAddBucketPartition() {
assertThat(table.spec()).as("Should have new spec
field").isEqualTo(expected);
}
+ @TestTemplate
+ public void testFailTypePromotionFieldReferencedInTransform() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+ createTable("id bigint NOT NULL, category string", "bucket(16, id)");
+ sql("INSERT INTO %s VALUES (1, 'cat1')", tableName);
+
+ assertThatThrownBy(() -> sql("ALTER TABLE %s ALTER COLUMN id TYPE string",
tableName))
+ .isInstanceOf(SparkException.class)
+ .hasMessageContaining(
+ "Cannot promote field 1 to string since it is part of a transform
that produces different values after promotion");
+ }
+
+ @TestTemplate
+ public void testAlterColumnStringPromotion() {
+ assumeThat(formatVersion).isGreaterThanOrEqualTo(3);
+ createTable("id bigint NOT NULL, category string");
+ sql("INSERT INTO %s VALUES (1, 'cat1')", tableName);
+ sql("ALTER TABLE %s ALTER COLUMN id TYPE string", tableName);
+ sql("INSERT INTO %s VALUES ('2', 'cat2')", tableName);
+
+ sql("REFRESH TABLE %s", tableName);
+
+ // Query based on a predicate where the actual value in the file is stored
as long
+ assertEquals(
+ "Should get first row",
+ ImmutableList.of(row("1", "cat1")),
+ sql("SELECT * FROM %s WHERE id='1'", tableName));
+
+ // Query based on a predicate where the actual value was written as an
integer
+ assertEquals(
+ "Should get second row",
+ ImmutableList.of(row("2", "cat2")),
+ sql("SELECT * FROM %s WHERE id='2'", tableName));
Review Comment:
Need more tests on different comparators in the predicates, nulls etc....
--
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]
