address comments

apache · Jan 14, 2025 · b871658 · b871658
1 parent 52a4dde
commit b871658
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 19 deletions.
diff --git a/flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl b/flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl
@@ -3336,8 +3336,8 @@ SqlAlterModel SqlAlterModel() :
             return new SqlAlterModelRename(
                         startPos.plus(getPos()),
                         modelIdentifier,
-                        ifExists,
-                        newModelIdentifier);
+                        newModelIdentifier,
+                        ifExists);
         }
     |
         <SET>

diff --git a/...k-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModel.java b/...k-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModel.java
@@ -30,7 +30,7 @@
 
 /**
  * Abstract class to describe statements like ALTER MODEL [IF EXISTS] [[catalogName.]
- * dataBasesName].modelName ...
+ * dataBasesName.]modelName ...
  */
 public abstract class SqlAlterModel extends SqlCall {
 

diff --git a/...e/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelRename.java b/...e/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelRename.java
@@ -28,16 +28,16 @@
 
 import static java.util.Objects.requireNonNull;
 
-/** ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName].modelName RENAME TO newModelName. */
+/** ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName.]modelName RENAME TO newModelName. */
 public class SqlAlterModelRename extends SqlAlterModel {
 
     private final SqlIdentifier newModelName;
 
     public SqlAlterModelRename(
             SqlParserPos pos,
             SqlIdentifier modelName,
-            boolean ifModelExists,
-            SqlIdentifier newModelName) {
+            SqlIdentifier newModelName,
+            boolean ifModelExists) {
         super(pos, modelName, ifModelExists);
         this.newModelName = requireNonNull(newModelName, "newModelName should not be null");
     }

diff --git a/...le/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelReset.java b/...le/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelReset.java
@@ -36,7 +36,7 @@
 import static java.util.Objects.requireNonNull;
 
 /**
- * ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName].modelName RESET ( 'key1' [, 'key2']...).
+ * ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName.]modelName RESET ( 'key1' [, 'key2']...).
  */
 public class SqlAlterModelReset extends SqlAlterModel {
     private final SqlNodeList optionKeyList;

diff --git a/...able/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelSet.java b/...able/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/ddl/SqlAlterModelSet.java
@@ -32,7 +32,7 @@
 import static java.util.Objects.requireNonNull;
 
 /**
- * ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName].modelName SET ( name=value [,
+ * ALTER MODEL [IF EXISTS] [[catalogName.] dataBasesName.]modelName SET ( name=value [,
  * name=value]*).
  */
 public class SqlAlterModelSet extends SqlAlterModel {

diff --git a/...c/test/java/org/apache/flink/table/planner/operations/SqlDdlToOperationConverterTest.java b/...c/test/java/org/apache/flink/table/planner/operations/SqlDdlToOperationConverterTest.java
@@ -51,8 +51,6 @@
 import org.apache.flink.table.operations.NopOperation;
 import org.apache.flink.table.operations.Operation;
 import org.apache.flink.table.operations.ShowModelsOperation;
-import org.apache.flink.table.operations.SinkModifyOperation;
-import org.apache.flink.table.operations.SourceQueryOperation;
 import org.apache.flink.table.operations.ddl.AddPartitionsOperation;
 import org.apache.flink.table.operations.ddl.AlterCatalogCommentOperation;
 import org.apache.flink.table.operations.ddl.AlterCatalogOptionsOperation;
@@ -118,7 +116,7 @@
 import static org.apache.flink.table.planner.utils.OperationMatchers.withSchema;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.junit.Assert.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 /** Test cases for the DDL statements for {@link SqlNodeToOperationConversion}. */
 public class SqlDdlToOperationConverterTest extends SqlNodeToOperationConversionTestBase {
@@ -428,9 +426,9 @@ public void testAlterModel() throws Exception {
 
         final String[] renameModelSqls =
                 new String[] {
-                    "alter model cat1.db1.m1 rename to m2",
-                    "alter model db1.m1 rename to m2",
-                    "alter model m1 rename to cat1.db1.m2",
+                    "ALTER MODEL cat1.db1.m1 RENAME to m2",
+                    "ALTER MODEL db1.m1 RENAME to m2",
+                    "ALTER MODEL m1 RENAME to cat1.db1.m2",
                 };
         final ObjectIdentifier expectedIdentifier = ObjectIdentifier.of("cat1", "db1", "m1");
         final ObjectIdentifier expectedNewIdentifier = ObjectIdentifier.of("cat1", "db1", "m2");
@@ -448,7 +446,7 @@ public void testAlterModel() throws Exception {
 
         // test alter model properties with existing key: 'k1'
         Operation operation =
-                parse("alter model if exists cat1.db1.m1 set ('k1' = 'v1_altered', 'K2' = 'V2')");
+                parse("ALTER MODEL IF EXISTS cat1.db1.m1 SET('k1' = 'v1_altered', 'K2' = 'V2')");
         assertThat(operation).isInstanceOf(AlterModelChangeOperation.class);
         AlterModelChangeOperation alterModelPropertiesOperation =
                 (AlterModelChangeOperation) operation;
@@ -460,7 +458,7 @@ public void testAlterModel() throws Exception {
         assertThat(alterModelPropertiesOperation.getModelChanges()).isEqualTo(expectedModelChanges);
 
         // test alter model properties without keys
-        operation = parse("alter model if exists cat1.db1.m1 set ('k3' = 'v3')");
+        operation = parse("ALTER MODEL IF EXISTS cat1.db1.m1 SET('k3' = 'v3')");
         expectedModelChanges.clear();
         expectedModelChanges.add(ModelChange.set("k3", "v3"));
 
@@ -471,12 +469,12 @@ public void testAlterModel() throws Exception {
         assertThat(alterModelPropertiesOperation.getModelChanges()).isEqualTo(expectedModelChanges);
 
         // test alter model properties empty option not supported
-        assertThatThrownBy(() -> parse("alter model if exists cat1.db1.m1 set ()"))
+        assertThatThrownBy(() -> parse("ALTER MODEL IF EXISTS cat1.db1.m1 SET()"))
                 .isInstanceOf(ValidationException.class)
                 .hasMessageContaining("ALTER MODEL SET does not support empty option");
 
         // test alter model reset properties with existing key: 'k1'
-        operation = parse("alter model if exists cat1.db1.m1 reset ('k1')");
+        operation = parse("ALTER MODEL IF EXISTS cat1.db1.m1 RESET('k1')");
         assertThat(operation).isInstanceOf(AlterModelChangeOperation.class);
         alterModelPropertiesOperation = (AlterModelChangeOperation) operation;
         assertThat(alterModelPropertiesOperation.getModelIdentifier())
@@ -486,7 +484,7 @@ public void testAlterModel() throws Exception {
         assertThat(alterModelPropertiesOperation.getModelChanges()).isEqualTo(expectedModelChanges);
 
         // test alter model reset properties empty key not supported
-        assertThatThrownBy(() -> parse("alter model if exists cat1.db1.m1 reset ()"))
+        assertThatThrownBy(() -> parse("ALTER MODEL IF EXISTS cat1.db1.m1 RESET()"))
                 .isInstanceOf(ValidationException.class)
                 .hasMessageContaining("ALTER MODEL RESET does not support empty key");
     }