Hisoka-X commented on code in PR #6679:
URL: https://github.com/apache/seatunnel/pull/6679#discussion_r1559294376
##########
seatunnel-connectors-v2/connector-doris/src/main/java/org/apache/seatunnel/connectors/doris/util/DorisCatalogUtil.java:
##########
@@ -110,21 +111,27 @@ public static String getCreateTableStatement(
String template = createTableTemplate;
TableSchema tableSchema = catalogTable.getTableSchema();
- String primaryKey = "";
+ String primaryKey = StringUtils.EMPTY;
if (tableSchema.getPrimaryKey() != null) {
primaryKey =
tableSchema.getPrimaryKey().getColumnNames().stream()
.map(r -> "`" + r + "`")
.collect(Collectors.joining(","));
}
- String uniqueKey = "";
+ String uniqueKey = StringUtils.EMPTY;
Review Comment:
ditto
##########
seatunnel-connectors-v2/connector-doris/src/main/java/org/apache/seatunnel/connectors/doris/util/DorisCatalogUtil.java:
##########
@@ -110,21 +111,27 @@ public static String getCreateTableStatement(
String template = createTableTemplate;
TableSchema tableSchema = catalogTable.getTableSchema();
- String primaryKey = "";
+ String primaryKey = StringUtils.EMPTY;
Review Comment:
I think the old code style more clear.
```suggestion
String primaryKey = "";
```
##########
seatunnel-connectors-v2/connector-doris/src/test/java/org/apache/seatunnel/connectors/doris/catalog/DorisCreateTableTest.java:
##########
@@ -130,6 +133,26 @@ public void test() {
+ "\"storage_format\" = \"V2\",\n"
+ "\"disable_auto_compaction\" = \"false\"\n"
+ ")");
+
+ CatalogTable catalogTable =
+ CatalogTable.of(
+ TableIdentifier.of("test", "test1", "test2"),
+ TableSchema.builder()
+ .primaryKey(
+ PrimaryKey.of(StringUtils.EMPTY,
Collections.emptyList()))
+ .constraintKey(Collections.emptyList())
+ .columns(columns)
+ .build(),
+ Collections.emptyMap(),
+ Collections.emptyList(),
+ "");
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ DorisCatalogUtil.getCreateTableStatement(
+
DorisOptions.SAVE_MODE_CREATE_TEMPLATE.defaultValue(),
+ TablePath.of("test1.test2"),
+ catalogTable));
Review Comment:
Please assert the `RuntimeException`'s message.
##########
seatunnel-connectors-v2/connector-doris/src/main/java/org/apache/seatunnel/connectors/doris/util/DorisCatalogUtil.java:
##########
@@ -153,6 +160,17 @@ public static String getCreateTableStatement(
rowTypeFields);
}
+ private static boolean checkPrimaryKeyAndUniqueKey(
Review Comment:
```suggestion
private static boolean canHandledByDefaultTemplate(
```
##########
seatunnel-connectors-v2/connector-doris/src/test/java/org/apache/seatunnel/connectors/doris/catalog/PreviewActionTest.java:
##########
@@ -110,10 +110,20 @@ private void assertPreviewResult(
Catalog.ActionType actionType,
String expectedSql,
Optional<CatalogTable> catalogTable) {
- PreviewResult previewResult =
- catalog.previewAction(
- actionType, TablePath.of("testddatabase.testtable"),
catalogTable);
- Assertions.assertInstanceOf(SQLPreviewResult.class, previewResult);
- Assertions.assertEquals(expectedSql, ((SQLPreviewResult)
previewResult).getSql());
+ if (!actionType.equals(Catalog.ActionType.CREATE_TABLE)) {
+ PreviewResult previewResult =
+ catalog.previewAction(
+ actionType,
TablePath.of("testddatabase.testtable"), catalogTable);
+ Assertions.assertInstanceOf(SQLPreviewResult.class, previewResult);
+ Assertions.assertEquals(expectedSql, ((SQLPreviewResult)
previewResult).getSql());
+ } else {
+ Assertions.assertThrows(
+ RuntimeException.class,
+ () ->
+ catalog.previewAction(
+ actionType,
+ TablePath.of("testddatabase.testtable"),
+ catalogTable));
+ }
Review Comment:
The file main to test preview create table action. So I think we should
modify test to make sure the test case of preview create table action passed,
not throw exception. I think we can add primary keys in `CATALOG_TABLE`.
##########
seatunnel-connectors-v2/connector-doris/src/main/java/org/apache/seatunnel/connectors/doris/util/DorisCatalogUtil.java:
##########
@@ -110,21 +111,27 @@ public static String getCreateTableStatement(
String template = createTableTemplate;
TableSchema tableSchema = catalogTable.getTableSchema();
- String primaryKey = "";
+ String primaryKey = StringUtils.EMPTY;
if (tableSchema.getPrimaryKey() != null) {
primaryKey =
tableSchema.getPrimaryKey().getColumnNames().stream()
.map(r -> "`" + r + "`")
.collect(Collectors.joining(","));
}
- String uniqueKey = "";
+ String uniqueKey = StringUtils.EMPTY;
if (!tableSchema.getConstraintKeys().isEmpty()) {
uniqueKey =
tableSchema.getConstraintKeys().stream()
.flatMap(c -> c.getColumnNames().stream())
.map(r -> "`" + r.getColumnName() + "`")
.collect(Collectors.joining(","));
}
+ if (checkPrimaryKeyAndUniqueKey(template, primaryKey, uniqueKey)) {
+ throw new RuntimeException(
+ String.format(
+ "The table of %s has no primaryKey or uniqueKey,
please use the option named save_mode_create_template to specify sql template",
Review Comment:
```suggestion
"The table of %s has no primaryKey or uniqueKey,
please use the option named " + SAVE_MODE_CREATE_TEMPLATE.key() + " to specify
sql template",
```
--
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]
