wuchong commented on code in PR #2617:
URL: https://github.com/apache/fluss/pull/2617#discussion_r2785534199
##########
fluss-client/src/test/java/org/apache/fluss/client/admin/FlussAdminITCase.java:
##########
@@ -1710,4 +1710,108 @@ void testAlterTableTieredLogLocalSegments() throws
Exception {
// 11. Cleanup
admin.dropTable(tablePath, false).get();
}
+
+ /**
+ * Test that aggregate merge engine tables cannot use WAL changelog image
mode.
+ *
+ * <p>Validates: Requirements 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 2.1, 2.2
+ */
+ @Test
+ void testCreateTableWithAggregateEngineAndWalChangelog() throws Exception {
+ // Schema for aggregate merge engine tables
+ Schema aggregateSchema =
+ Schema.newBuilder()
+ .column("id", DataTypes.INT())
+ .column("count", DataTypes.BIGINT(),
AggFunctions.SUM())
+ .primaryKey("id")
+ .build();
+
+ // Test 1: Aggregate + WAL should be rejected
+ TablePath tablePath1 = TablePath.of("fluss",
"test_aggregate_wal_changelog_rejected");
+ Map<String, String> properties1 = new HashMap<>();
+ properties1.put(ConfigOptions.TABLE_MERGE_ENGINE.key(), "aggregation");
+ properties1.put(ConfigOptions.TABLE_CHANGELOG_IMAGE.key(), "WAL");
+ TableDescriptor tableDescriptor1 =
+ TableDescriptor.builder()
+ .schema(aggregateSchema)
+ .comment("aggregate merge engine table with WAL
changelog")
+ .properties(properties1)
+ .build();
+ assertThatThrownBy(() -> admin.createTable(tablePath1,
tableDescriptor1, false).get())
+ .cause()
+ .isInstanceOf(InvalidConfigException.class)
+ .hasMessageContaining(ConfigOptions.TABLE_MERGE_ENGINE.key())
+
.hasMessageContaining(ConfigOptions.TABLE_CHANGELOG_IMAGE.key());
Review Comment:
Actually, I prefer to assert the **full error message** to ensure that the
error we throw is both **user-friendly and correctly formatted**. Otherwise,
we’re not fully validating the exception message, which could allow unclear or
malformed errors to go unnoticed.
##########
fluss-server/src/main/java/org/apache/fluss/server/utils/TableDescriptorValidation.java:
##########
@@ -302,6 +303,18 @@ private static void checkMergeEngine(
versionColumn.get(), columnType));
}
} else if (mergeEngine == MergeEngineType.AGGREGATION) {
+ // Check aggregate merge engine with WAL changelog image
+ ChangelogImage changelogImage =
tableConf.get(ConfigOptions.TABLE_CHANGELOG_IMAGE);
+ if (changelogImage == ChangelogImage.WAL) {
+ throw new InvalidConfigException(
+ String.format(
+ "Table with '%s' merge engine does not
support '%s' changelog image mode. "
Review Comment:
I think we can hardcode the merge engine type and changelog image mode here,
rather than print the config key?
--
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]
