arkadius commented on code in PR #11249:
URL: https://github.com/apache/iceberg/pull/11249#discussion_r1827996850
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/sink/TestIcebergSinkV2.java:
##########
@@ -107,15 +153,16 @@ public void testCheckAndGetEqualityFieldIds() {
.setIdentifierFields("type")
.commit();
- DataStream<Row> dataStream =
- env.addSource(new BoundedTestSource<>(ImmutableList.of()),
ROW_TYPE_INFO);
- IcebergSink.Builder builder =
- IcebergSink.forRow(dataStream,
SimpleDataUtil.FLINK_SCHEMA).table(table);
+ // Use schema identifier field IDs as equality field id list by default
+ assertThat(SinkUtil.checkAndGetEqualityFieldIds(table, null))
+
.containsExactlyInAnyOrderElementsOf(table.schema().identifierFieldIds());
// Use user-provided equality field column as equality field id list
- builder.equalityFieldColumns(Lists.newArrayList("id"));
assertThat(SinkUtil.checkAndGetEqualityFieldIds(table,
Lists.newArrayList("id")))
.containsExactlyInAnyOrder(table.schema().findField("id").fieldId());
+
+ assertThat(SinkUtil.checkAndGetEqualityFieldIds(table,
Lists.newArrayList("type")))
+ .containsExactlyInAnyOrder(table.schema().findField("type").fieldId());
Review Comment:
`TestIcebergSinkV2` is a copy of `TestFlinkIcebergSinkV2` (which tests
V1-based `FlinkSink`). For some reasons, these checks weren't copied. At first
glance it looks like most of tests is this class don't check anything related
with SinkV2. Take a look that most of test cases use
`TestFlinkIcebergSinkV2Base.testChangeLogs` which uses `FlinkSink` (V1 based).
My proposition of plan is:
1. Synchronize the code duplication
2. Replace duplicated classes with parametrized test case
3. Extract things that are not related with either V1 or V2 sinks to some
other classes as you suggested
WDYT?
--
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
