alex-plekhanov commented on a change in pull request #9185:
URL: https://github.com/apache/ignite/pull/9185#discussion_r672185927



##########
File path: 
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/SchemaManager.java
##########
@@ -0,0 +1,65 @@
+package org.apache.ignite.internal.processors.query.calcite.exec.ddl;

Review comment:
       Fixed, thanks

##########
File path: 
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/AlterTableDropCommand.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.prepare.ddl;
+
+import java.util.List;
+
+/**
+ * ALTER TABLE ... ADD COLUMN statement.

Review comment:
       Fixed

##########
File path: 
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/AlterTableDropCommand.java
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.prepare.ddl;
+
+import java.util.List;
+
+/**
+ * ALTER TABLE ... ADD COLUMN statement.
+ */
+@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType")
+public class AlterTableDropCommand implements DdlCommand {
+    /** Schema name. */
+    private String schemaName;
+
+    /** Table name. */
+    private String tblName;
+
+    /** Quietly ignore this command if table is not exists. */
+    private boolean ifTableExists;
+
+    /** Quietly ignore this command if column is not exists. */
+    private boolean ifColumnExists;
+
+    /** Columns. */
+    private List<String> cols;
+
+    /**
+     * @return Columns.
+     */
+    public List<String> columns() {
+        return cols;

Review comment:
       I think It's not necessary, since it is used only in one place, but fixed

##########
File path: 
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -226,6 +234,64 @@ private DropTableCommand convertDropTable(SqlDropTable 
dropTblNode, PlanningCont
         return dropTblCmd;
     }
 
+    /**
+     * Converts a given IgniteSqlAlterTableAddColumn AST to a 
AlterTableAddCommand.
+     *
+     * @param alterTblNode Root node of the given AST.
+     * @param ctx Planning context.
+     */
+    private AlterTableAddCommand 
convertAlterTableAdd(IgniteSqlAlterTableAddColumn alterTblNode, PlanningContext 
ctx) {
+        AlterTableAddCommand alterTblCmd = new AlterTableAddCommand();
+
+        alterTblCmd.schemaName(deriveSchemaName(alterTblNode.name(), ctx));
+        alterTblCmd.tableName(deriveObjectName(alterTblNode.name(), ctx, 
"table name"));
+        alterTblCmd.ifTableExists(alterTblNode.ifExists());
+        alterTblCmd.ifColumnNotExists(alterTblNode.ifNotExistsColumn());
+
+        List<ColumnDefinition> cols = new ArrayList<>();

Review comment:
       Fixed

##########
File path: 
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java
##########
@@ -337,17 +338,260 @@ public void dropTableIfExists() {
 
         GridTestUtils.assertThrows(log,
             () -> executeSql("drop table my_table"),
-            IgniteSQLException.class, "Table doesn't exist: MY_TABLE]");
+            IgniteSQLException.class, "Table doesn't exist: MY_TABLE");
 
         executeSql("drop table my_schema.my_table");
 
         GridTestUtils.assertThrows(log,
             () -> executeSql("drop table my_schema.my_table"),
-            IgniteSQLException.class, "Table doesn't exist: MY_TABLE]");
+            IgniteSQLException.class, "Table doesn't exist: MY_TABLE");
 
         executeSql("drop table if exists my_schema.my_table");
     }
 
+    /**
+     * Add/drop column simple case.
+     */
+    @Test
+    public void alterTableAddDropSimpleCase() {
+        executeSql("create table my_table (id int primary key, val varchar)");
+
+        executeSql("alter table my_table add column val2 varchar");
+
+        executeSql("insert into my_table (id, val, val2) values (0, '1', 
'2')");
+
+        List<List<?>> res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals("2", res.get(0).get(2));
+
+        executeSql("alter table my_table drop column val2");
+
+        res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals(2, res.get(0).size());
+    }
+
+    /**
+     * Add/drop two columns at the same time.
+     */
+    @Test
+    public void alterTableAddDropTwoColumns() {
+        executeSql("create table my_table (id int primary key, val varchar)");
+
+        executeSql("alter table my_table add column (val2 varchar, val3 int)");
+
+        executeSql("insert into my_table (id, val, val2, val3) values (0, '1', 
'2', 3)");
+
+        List<List<?>> res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals("2", res.get(0).get(2));
+        assertEquals(3, res.get(0).get(3));
+
+        executeSql("alter table my_table drop column (val2, val3)");
+
+        res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals(2, res.get(0).size());
+    }
+
+    /**
+     * Add/drop column for table in custom schema.
+     */
+    @Test
+    public void alterTableAddDropCustomSchema() {
+        executeSql("create table my_schema.my_table (id int primary key, val 
varchar)");
+
+        executeSql("alter table my_schema.my_table add column val2 varchar");
+
+        executeSql("insert into my_schema.my_table (id, val, val2) values (0, 
'1', '2')");
+
+        List<List<?>> res = executeSql("select * from my_schema.my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals("2", res.get(0).get(2));
+
+        executeSql("alter table my_schema.my_table drop column val2");
+
+        res = executeSql("select * from my_schema.my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals(2, res.get(0).size());
+    }
+
+    /**
+     * Add/drop column if table exists.
+     */
+    @Test
+    public void alterTableAddDropIfTableExists() {
+        assertThrows("alter table my_table add val2 varchar", 
IgniteSQLException.class, "Table doesn't exist");
+
+        executeSql("alter table if exists my_table add column val2 varchar");
+
+        assertThrows("alter table my_table drop column val2", 
IgniteSQLException.class, "Table doesn't exist");
+
+        executeSql("alter table if exists my_table drop column val2");
+
+        executeSql("create table my_table (id int primary key, val varchar)");
+
+        executeSql("alter table if exists my_table add column val3 varchar");
+
+        executeSql("insert into my_table (id, val, val3) values (0, '1', 
'2')");
+
+        List<List<?>> res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals("2", res.get(0).get(2));
+
+        executeSql("alter table if exists my_table drop column val3");
+
+        assertThrows("alter table if exists my_table drop column val3", 
IgniteSQLException.class,
+            "Column doesn't exist");
+
+        res = executeSql("select * from my_table ");
+
+        assertEquals(1, res.size());
+        assertEquals(2, res.get(0).size());
+    }
+

Review comment:
       Tests added

##########
File path: 
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -226,6 +234,64 @@ private DropTableCommand convertDropTable(SqlDropTable 
dropTblNode, PlanningCont
         return dropTblCmd;
     }
 
+    /**
+     * Converts a given IgniteSqlAlterTableAddColumn AST to a 
AlterTableAddCommand.
+     *
+     * @param alterTblNode Root node of the given AST.
+     * @param ctx Planning context.
+     */
+    private AlterTableAddCommand 
convertAlterTableAdd(IgniteSqlAlterTableAddColumn alterTblNode, PlanningContext 
ctx) {
+        AlterTableAddCommand alterTblCmd = new AlterTableAddCommand();
+
+        alterTblCmd.schemaName(deriveSchemaName(alterTblNode.name(), ctx));
+        alterTblCmd.tableName(deriveObjectName(alterTblNode.name(), ctx, 
"table name"));
+        alterTblCmd.ifTableExists(alterTblNode.ifExists());
+        alterTblCmd.ifColumnNotExists(alterTblNode.ifNotExistsColumn());
+
+        List<ColumnDefinition> cols = new ArrayList<>();
+
+        for (SqlNode colNode : alterTblNode.columns()) {
+            assert colNode instanceof SqlColumnDeclaration : 
colNode.getClass();
+
+            SqlColumnDeclaration col = (SqlColumnDeclaration)colNode;
+
+            assert col.name.isSimple();
+
+            String name = col.name.getSimple();
+            RelDataType type = ctx.planner().convert(col.dataType);
+
+            assert col.expression == null : "Unexpected column default value" 
+ col.expression;
+
+            cols.add(new ColumnDefinition(name, type, null));
+        }
+
+        alterTblCmd.columns(cols);
+
+        return alterTblCmd;
+    }
+
+    /**
+     * Converts a given IgniteSqlAlterTableDropColumn AST to a 
AlterTableDropCommand.
+     *
+     * @param alterTblNode Root node of the given AST.
+     * @param ctx Planning context.
+     */
+    private AlterTableDropCommand 
convertAlterTableDrop(IgniteSqlAlterTableDropColumn alterTblNode, 
PlanningContext ctx) {
+        AlterTableDropCommand alterTblCmd = new AlterTableDropCommand();
+
+        alterTblCmd.schemaName(deriveSchemaName(alterTblNode.name(), ctx));
+        alterTblCmd.tableName(deriveObjectName(alterTblNode.name(), ctx, 
"table name"));
+        alterTblCmd.ifTableExists(alterTblNode.ifExists());
+        alterTblCmd.ifColumnExists(alterTblNode.ifExistsColumn());
+
+        List<String> cols = new ArrayList<>();

Review comment:
       Fixed




-- 
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]


Reply via email to