rodmeneses commented on code in PR #10112:
URL: https://github.com/apache/iceberg/pull/10112#discussion_r1561381139
##########
flink/v1.19/flink/src/test/java/org/apache/iceberg/flink/source/TestStreamingMonitorFunction.java:
##########
@@ -248,6 +248,7 @@ public void testInvalidMaxPlanningSnapshotCount() {
.monitorInterval(Duration.ofMillis(100))
.maxPlanningSnapshotCount(0)
.build();
+
Review Comment:
please see my comment above
##########
flink/v1.19/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkInputFormatReaderDeletes.java:
##########
@@ -35,6 +35,7 @@
import org.apache.iceberg.util.StructLikeSet;
public class TestFlinkInputFormatReaderDeletes extends
TestFlinkReaderDeletesBase {
+
Review Comment:
please see my comment above
##########
flink/v1.19/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java:
##########
@@ -294,39 +308,284 @@ public void testAlterTable() throws
TableNotExistException {
assertThat(table("tl").properties()).containsAllEntriesOf(properties);
// remove property
- CatalogTable catalogTable = catalogTable("tl");
+ sql("ALTER TABLE tl RESET('oldK')");
properties.remove("oldK");
- getTableEnv()
- .getCatalog(getTableEnv().getCurrentCatalog())
- .get()
- .alterTable(new ObjectPath(DATABASE, "tl"),
catalogTable.copy(properties), false);
assertThat(table("tl").properties()).containsAllEntriesOf(properties);
}
@TestTemplate
- public void testAlterTableWithPrimaryKey() throws TableNotExistException {
- sql("CREATE TABLE tl(id BIGINT, PRIMARY KEY(id) NOT ENFORCED) WITH
('oldK'='oldV')");
- Map<String, String> properties = Maps.newHashMap();
- properties.put("oldK", "oldV");
+ public void testAlterTableAddColumn() {
+ sql("CREATE TABLE tl(id BIGINT)");
+ Schema schemaBefore = table("tl").schema();
+ assertThat(schemaBefore.asStruct())
+ .isEqualTo(
+ new Schema(Types.NestedField.optional(1, "id",
Types.LongType.get())).asStruct());
+ sql("ALTER TABLE tl ADD (dt STRING)");
+ Schema schemaAfter1 = table("tl").schema();
+ assertThat(schemaAfter1.asStruct())
+ .isEqualTo(
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "dt",
Types.StringType.get()))
+ .asStruct());
+ // Add multiple columns
+ sql("ALTER TABLE tl ADD (col1 STRING COMMENT 'comment for col1', col2
BIGINT)");
+ Schema schemaAfter2 = table("tl").schema();
+ assertThat(schemaAfter2.asStruct())
+ .isEqualTo(
+ new Schema(
+ Types.NestedField.optional(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "dt",
Types.StringType.get()),
+ Types.NestedField.optional(
+ 3, "col1", Types.StringType.get(), "comment for col1"),
+ Types.NestedField.optional(4, "col2",
Types.LongType.get()))
+ .asStruct());
+ // Adding a required field should fail because Iceberg's SchemaUpdate does
not allow
+ // incompatible changes.
+ Assertions.assertThatThrownBy(() -> sql("ALTER TABLE tl ADD (pk STRING NOT
NULL)"))
Review Comment:
please see my comment above
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
