Akash3121 commented on code in PR #10077:
URL: https://github.com/apache/paimon/pull/10077#discussion_r4077885494
##########
paimon-core/src/test/java/org/apache/paimon/schema/SchemaMergingUtilsTest.java:
##########
@@ -144,6 +144,80 @@ public void testMergeSchemasWithoutTypeWidening() {
assertThat(fields.get(2).type()).isEqualTo(new IntType());
}
+ @Test
+ public void testRejectTypeWideningOnPrimaryKeyColumn() {
+ // An INT primary key must not be widened to BIGINT by the automatic
merge path: INT
+ // values hash into different buckets than their BIGINT counterparts,
so the same logical
+ // key would end up in two buckets and bypass intra-bucket
deduplication.
+ DataField a = new DataField(0, "a", new IntType());
+ DataField b = new DataField(1, "b", new
VarCharType(VarCharType.MAX_LENGTH));
+ TableSchema current =
+ new TableSchema(
+ 0,
+ Lists.newArrayList(a, b),
+ 1,
+ new ArrayList<>(),
+ Lists.newArrayList("a"),
+ new HashMap<>(),
+ "");
+
+ DataField aWidened = new DataField(-1, "a", new BigIntType());
+ RowType t = new RowType(Lists.newArrayList(aWidened, b));
+
+ assertThatThrownBy(() -> SchemaMergingUtils.mergeSchemas(current, t,
true, false, true))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining("Cannot update primary key type");
+ }
+
+ @Test
+ public void testRejectTypeWideningOnPartitionColumn() {
+ // Partition columns are equally encoded into the partition path;
widening them would
+ // silently create a second partition layout for the same logical key.
+ DataField a = new DataField(0, "a", new IntType());
+ DataField b = new DataField(1, "b", new
VarCharType(VarCharType.MAX_LENGTH));
+ TableSchema current =
+ new TableSchema(
+ 0,
+ Lists.newArrayList(a, b),
+ 1,
+ Lists.newArrayList("b"),
+ new ArrayList<>(),
+ new HashMap<>(),
+ "");
+
+ DataField bWidened = new DataField(-1, "b", new VarCharType(100));
Review Comment:
This test is named `testRejectTypeWideningOnPartitionColumn` , but it
changes `VARCHAR(MAX)` to `VARCHAR(100)` , which is a narrowing conversion,
and it only succeeds as a merge candidate because `allowExplicitCast` is
enabled below. Could we instead use INT -> BIGINT for the partition column
and call `mergeSchemas` with `allowExplicitCast=false` ? That would directly
verify that the normal type-widening path rejects partition-column type
changes, independently of explicit-cast behavior.
##########
paimon-core/src/test/java/org/apache/paimon/schema/SchemaMergingUtilsTest.java:
##########
@@ -144,6 +144,80 @@ public void testMergeSchemasWithoutTypeWidening() {
assertThat(fields.get(2).type()).isEqualTo(new IntType());
}
+ @Test
+ public void testRejectTypeWideningOnPrimaryKeyColumn() {
+ // An INT primary key must not be widened to BIGINT by the automatic
merge path: INT
+ // values hash into different buckets than their BIGINT counterparts,
so the same logical
+ // key would end up in two buckets and bypass intra-bucket
deduplication.
+ DataField a = new DataField(0, "a", new IntType());
+ DataField b = new DataField(1, "b", new
VarCharType(VarCharType.MAX_LENGTH));
+ TableSchema current =
+ new TableSchema(
+ 0,
+ Lists.newArrayList(a, b),
+ 1,
+ new ArrayList<>(),
+ Lists.newArrayList("a"),
+ new HashMap<>(),
+ "");
+
+ DataField aWidened = new DataField(-1, "a", new BigIntType());
+ RowType t = new RowType(Lists.newArrayList(aWidened, b));
+
+ assertThatThrownBy(() -> SchemaMergingUtils.mergeSchemas(current, t,
true, false, true))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining("Cannot update primary key type");
+ }
+
+ @Test
+ public void testRejectTypeWideningOnPartitionColumn() {
+ // Partition columns are equally encoded into the partition path;
widening them would
+ // silently create a second partition layout for the same logical key.
+ DataField a = new DataField(0, "a", new IntType());
+ DataField b = new DataField(1, "b", new
VarCharType(VarCharType.MAX_LENGTH));
+ TableSchema current =
+ new TableSchema(
+ 0,
+ Lists.newArrayList(a, b),
+ 1,
+ Lists.newArrayList("b"),
+ new ArrayList<>(),
+ new HashMap<>(),
+ "");
+
+ DataField bWidened = new DataField(-1, "b", new VarCharType(100));
Review Comment:
The corresponding corrected test setup would look like:
```java
DataField b = new DataField(1, "b", new IntType());
// ...
DataField bWidened = new DataField(-1, "b", new BigIntType());
RowType t = new RowType(Lists.newArrayList(a, bWidened));
assertThatThrownBy(() -> SchemaMergingUtils.mergeSchemas(current, t, true,
false, true))
.isInstanceOf(UnsupportedOperationException.class)
.hasMessageContaining("Cannot update partition column type");
```
--
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]
