korlov42 commented on a change in pull request #484:
URL: https://github.com/apache/ignite-3/pull/484#discussion_r766654600
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -209,6 +210,68 @@ private CreateTableCommand
convertCreateTable(IgniteSqlCreateTable createTblNode
return createTblCmd;
}
+ /**
+ * 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.ifTableNotExists(alterTblNode.ifExists());
Review comment:
Sorry, but I can't stand this :( If you want to derive an abstraction,
you have to find a suitable name for an abstract fields. Current approach could
easily mislead someone.
##########
File path:
modules/api/src/main/java/org/apache/ignite/lang/IndexNotFoundException.java
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.lang;
+
+/**
+ * Exception is thrown when appropriate index is not found.
+ */
+public class IndexNotFoundException extends IgniteException {
+ /**
+ * Create a new exception with given index name.
+ *
+ * @param indexName Index name.
Review comment:
```suggestion
* @param indexName Index canonical name.
```
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -220,11 +283,56 @@ private DropTableCommand convertDropTable(SqlDropTable
dropTblNode, PlanningCont
dropTblCmd.schemaName(deriveSchemaName(dropTblNode.name, ctx));
dropTblCmd.tableName(deriveObjectName(dropTblNode.name, ctx,
"tableName"));
- dropTblCmd.ifExists(dropTblNode.ifExists);
+ dropTblCmd.ifTableNotExists(dropTblNode.ifExists);
Review comment:
how could we drop table that actually not exists?
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/IgniteTypeFactory.java
##########
@@ -136,8 +140,35 @@ public Type getJavaClass(RelDataType type) {
}
/**
- * Get result type by field data type.
- * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
+ * Gets ColumnType type for given class.
+ *
+ * @param relType Rel type.
+ * @return ColumnType type or null.
+ */
+ public ColumnType columnType(RelDataType relType) {
+ assert relType != null;
+
+ Type javaType = getResultClass(relType);
+
+ if (javaType == byte[].class) {
+ return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.blobOf() :
+ ColumnType.blobOf(relType.getPrecision());
+ } else if (javaType == String.class) {
+ return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.string() :
+ ColumnType.stringOf(relType.getPrecision());
+ } else if (javaType == BigInteger.class) {
+ return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.numberOf() :
+ ColumnType.numberOf(relType.getPrecision());
+ } else if (javaType == BigDecimal.class) {
+ return relType.getPrecision() == PRECISION_NOT_SPECIFIED ?
ColumnType.decimalOf() :
+ ColumnType.decimalOf(relType.getPrecision(),
relType.getScale());
Review comment:
oh, sorry for misleading you. I though it would be
`RelDataType#SCALE_NOT_SPECIFIED` which is `Integer.MIN_VALUE`
##########
File path: modules/calcite/src/main/codegen/includes/parserImpls.ftl
##########
@@ -184,6 +220,23 @@ SqlDrop SqlDropTable(Span s, boolean replace) :
}
}
+SqlDrop SqlDropIndex(Span s, boolean replace) :
+{
+ final boolean ifExists;
+ final SqlIdentifier idxId;
+ final SqlIdentifier tblId;
+}
+{
+ <INDEX>
+ ifExists = IfExistsOpt()
+ idxId = SimpleIdentifier()
+ <ON>
Review comment:
I believe this could be resolved on sql2cmd conversion phase. With
canonical index name you could find this index in a catalog reader. A schema
object representing index should contain a reference to a related table.
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.exec.ddl;
+
+import static
org.apache.ignite.internal.schema.configuration.SchemaConfigurationConverter.convert;
+import static org.apache.ignite.internal.util.CollectionUtils.nullOrEmpty;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.NamedListView;
+import org.apache.ignite.configuration.schemas.table.ColumnView;
+import org.apache.ignite.configuration.schemas.table.PrimaryKeyView;
+import org.apache.ignite.configuration.schemas.table.TableChange;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.PlanningContext;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.AbstractTableDdlCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.AlterTableAddCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.AlterTableDropCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.CreateIndexCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.CreateTableCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.DdlCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.DropIndexCommand;
+import
org.apache.ignite.internal.processors.query.calcite.prepare.ddl.DropTableCommand;
+import org.apache.ignite.internal.schema.definition.TableDefinitionImpl;
+import org.apache.ignite.internal.table.distributed.TableManager;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.Pair;
+import org.apache.ignite.lang.ColumnAlreadyExistsException;
+import org.apache.ignite.lang.ColumnNotFoundException;
+import org.apache.ignite.lang.IgniteException;
+import org.apache.ignite.lang.IgniteInternalCheckedException;
+import org.apache.ignite.lang.IndexAlreadyExistsException;
+import org.apache.ignite.lang.IndexNotFoundException;
+import org.apache.ignite.lang.LoggerMessageHelper;
+import org.apache.ignite.schema.SchemaBuilders;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.builder.PrimaryKeyDefinitionBuilder;
+import
org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
+import
org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
+
+/** DDL commands handler. */
+public class DdlCommandHandler {
+ private final TableManager tableManager;
+
+ public DdlCommandHandler(TableManager tblManager) {
+ tableManager = tblManager;
+ }
+
+ /** Handles ddl commands. */
+ public void handle(DdlCommand cmd, PlanningContext pctx) throws
IgniteInternalCheckedException {
+ validateCommand(cmd);
+
+ if (cmd instanceof CreateTableCommand) {
+ handleCreateTable((CreateTableCommand) cmd);
+ } else if (cmd instanceof DropTableCommand) {
+ handleDropTable((DropTableCommand) cmd);
+ } else if (cmd instanceof AlterTableAddCommand) {
+ handleAlterAddColumn((AlterTableAddCommand) cmd);
+ } else if (cmd instanceof AlterTableDropCommand) {
+ handleAlterDropColumn((AlterTableDropCommand) cmd);
+ } else if (cmd instanceof CreateIndexCommand) {
+ handleCreateIndex((CreateIndexCommand) cmd);
+ } else if (cmd instanceof DropIndexCommand) {
+ handleDropIndex((DropIndexCommand) cmd);
+ } else {
+ throw new IgniteInternalCheckedException("Unsupported DDL
operation ["
+ + "cmdName=" + (cmd == null ? null :
cmd.getClass().getSimpleName()) + "; "
+ + "querySql=\"" + pctx.query() + "\"]");
+ }
+ }
+
+ /** Validate command. */
+ private void validateCommand(DdlCommand cmd) {
+ if (cmd instanceof AbstractTableDdlCommand) {
+ AbstractTableDdlCommand cmd0 = (AbstractTableDdlCommand) cmd;
+
+ if (IgniteUtils.nullOrEmpty(cmd0.tableName())) {
+ throw new IllegalArgumentException("Table name is undefined.");
+ }
+ }
+ }
+
+ /** Handles create table command. */
+ private void handleCreateTable(CreateTableCommand cmd) {
+ PrimaryKeyDefinitionBuilder pkeyDef = SchemaBuilders.primaryKey();
+ pkeyDef.withColumns(cmd.primaryKeyColumns());
+ pkeyDef.withAffinityColumns(cmd.affColumns());
+
+ Consumer<TableChange> tblChanger = tblCh -> {
+ TableChange conv =
convert(SchemaBuilders.tableBuilder(cmd.schemaName(), cmd.tableName())
+ .columns(cmd.columns())
+ .withPrimaryKey(pkeyDef.build()).build(), tblCh);
+
+ if (cmd.partitions() != null) {
+ conv.changePartitions(cmd.partitions());
+ }
+
+ if (cmd.replicas() != null) {
+ conv.changeReplicas(cmd.replicas());
+ }
+ };
+
+ String fullName = TableDefinitionImpl.canonicalName(cmd.schemaName(),
cmd.tableName());
+
+ if (cmd.ifTableNotExists()) {
+ tableManager.createTableIfNotExists(fullName, tblChanger);
+ } else {
+ tableManager.createTable(fullName, tblChanger);
+ }
+ }
+
+ /** Handles drop table command. */
+ private void handleDropTable(DropTableCommand cmd) {
+ String fullName = TableDefinitionImpl.canonicalName(cmd.schemaName(),
cmd.tableName());
+
+ // if (!cmd.ifTableExists()) todo will be implemented after
IGNITE-15926
Review comment:
Mentioned issue is not valid anymore. We need to catch an exception here
and just ignore it if needed.
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -157,25 +153,30 @@ private CreateTableCommand
convertCreateTable(IgniteSqlCreateTable createTblNode
.collect(Collectors.toList());
IgnitePlanner planner = ctx.planner();
+ IgniteTypeFactory typeFactory = ctx.typeFactory();
- List<ColumnDefinition> cols = new ArrayList<>();
+ List<ColumnDefinition> cols = new ArrayList<>(colDeclarations.size());
for (SqlColumnDeclaration col : colDeclarations) {
if (!col.name.isSimple()) {
throw new IgniteException("Unexpected value of columnName ["
+ "expected a simple identifier, but was " + col.name
+ "; "
- + "querySql=\"" + ctx.query() + "\"]"/*,
IgniteQueryErrorCode.PARSING*/);
+ + "querySql=\"" + ctx.query() + "\"]");
}
String name = col.name.getSimple();
- RelDataType type = planner.convert(col.dataType);
+ RelDataType relType = planner.convert(col.dataType);
Object dflt = null;
if (col.expression != null) {
dflt = ((SqlLiteral) col.expression).getValue();
}
- cols.add(new ColumnDefinition(name, type, dflt));
+ ColumnDefinitionBuilder col0 = SchemaBuilders.column(name,
typeFactory.columnType(relType))
Review comment:
ColumnDefinition serves one small yet crucial purpose -- it narrows an
integration point to DdlCommandHandler only. I believe that such a tight
coupling across several layers does more harm than an extra conversion
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/CreateIndexCommand.java
##########
@@ -0,0 +1,99 @@
+/*
+ * 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;
+import org.apache.ignite.internal.util.Pair;
+
+/**
+ * CREATE INDEX statement.
+ */
+public class CreateIndexCommand implements DdlCommand {
+ /** Table name. */
+ private String tblName;
+
+ /** Quietly ignore this command if table is not exists. */
+ protected boolean ifTableNotExists;
+
+ /**
+ * Schema name upon which this statement has been issued - <b>not</b> the
name of the schema where this new table will be created.
+ * i.e. in case of: CREATE TABLE "SCH1"."TABL1" ... schema would be "SCH1".
+ */
Review comment:
This comment a bit contradictory. The schema this command is issued upon
is PUBLIC (I suppose), and the schema the table will be created is "SCH1", but
the example say the expected schema is "SCH1".
Looks like this schema is actual schema this index will be created in.
The same for other command.
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java
##########
@@ -209,6 +210,68 @@ private CreateTableCommand
convertCreateTable(IgniteSqlCreateTable createTblNode
return createTblCmd;
}
+ /**
+ * 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.ifTableNotExists(alterTblNode.ifExists());
+ alterTblCmd.ifColumnNotExists(alterTblNode.ifNotExistsColumn());
+
+ IgniteTypeFactory typeFactory = ctx.typeFactory();
+
+ List<ColumnDefinition> cols = new
ArrayList<>(alterTblNode.columns().size());
+
+ for (SqlNode colNode : alterTblNode.columns()) {
+ assert colNode instanceof SqlColumnDeclaration :
colNode.getClass();
+
+ SqlColumnDeclaration col = (SqlColumnDeclaration) colNode;
+
+ assert col.name.isSimple();
+ assert col.expression == null : "Unexpected column default value"
+ col.expression;
Review comment:
why do we forbid adding columns with default value?
--
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]
