This is an automated email from the ASF dual-hosted git repository.

amashenkov pushed a commit to branch ignite-17580
in repository https://gitbox.apache.org/repos/asf/ignite-3.git

commit e8d51dd54d97da3acf18c47bdc601712c3b2d7d9
Author: amashenkov <[email protected]>
AuthorDate: Mon Aug 29 17:47:54 2022 +0300

    Rewrite create index operation replacing definition objects to config 
changers.
---
 .../ignite/schema/definition/index/SortOrder.java  | 17 ++++-
 .../schema/configuration/TableValidatorImpl.java   |  4 +-
 .../schema/definition/SchemaValidationUtils.java   | 77 ++++++++++++++++++++++
 .../schema/utils/builder/AbstractIndexBuilder.java |  1 -
 .../builder/SortedIndexDefinitionBuilder.java      | 14 ++++
 .../builder/SortedIndexDefinitionBuilderImpl.java  | 35 ++++++++--
 .../utils/builder/TableDefinitionBuilderImpl.java  | 56 ++--------------
 .../sql/engine/exec/ddl/DdlCommandHandler.java     | 41 +++++++-----
 8 files changed, 168 insertions(+), 77 deletions(-)

diff --git 
a/modules/api/src/main/java/org/apache/ignite/schema/definition/index/SortOrder.java
 
b/modules/api/src/main/java/org/apache/ignite/schema/definition/index/SortOrder.java
index 2bf72fbe62..0c9b4b0ee1 100644
--- 
a/modules/api/src/main/java/org/apache/ignite/schema/definition/index/SortOrder.java
+++ 
b/modules/api/src/main/java/org/apache/ignite/schema/definition/index/SortOrder.java
@@ -39,16 +39,31 @@ public enum SortOrder {
     /** Nulls-first flag. */
     private final boolean nullsFirst;
 
-    /**  */
+    /**
+     * Constructor.
+     *
+     * @param asc Ascending order flag.
+     * @param nullsFirst Nulls first order flag.
+     */
     private SortOrder(boolean asc, boolean nullsFirst) {
         this.asc = asc;
         this.nullsFirst = nullsFirst;
     }
 
+    /**
+     * Return value order flag.
+     *
+     * @return {@code true} for ascending order, {@code false} for descending 
order.
+     */
     public boolean asc() {
         return asc;
     }
 
+    /**
+     * Return nulls order flag.
+     *
+     * @return {@code true} for nulls-first order, {@code false} for 
nulls-last order.
+     */
     public boolean nullsFirst() {
         return nullsFirst;
     }
diff --git 
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
 
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
index 6d4cc9fa6a..a9da01df30 100644
--- 
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
+++ 
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
@@ -23,8 +23,8 @@ import 
org.apache.ignite.configuration.schemas.table.TableView;
 import org.apache.ignite.configuration.validation.ValidationContext;
 import org.apache.ignite.configuration.validation.ValidationIssue;
 import org.apache.ignite.configuration.validation.Validator;
+import org.apache.ignite.internal.schema.definition.SchemaValidationUtils;
 import org.apache.ignite.internal.schema.definition.TableDefinitionImpl;
-import 
org.apache.ignite.internal.schema.definition.builder.TableDefinitionBuilderImpl;
 
 /**
  * Table schema configuration validator implementation.
@@ -47,7 +47,7 @@ public class TableValidatorImpl implements 
Validator<TableValidator, NamedListVi
                 assert !tbl.keyColumns().isEmpty();
                 assert !tbl.colocationColumns().isEmpty();
 
-                TableDefinitionBuilderImpl.validateIndices(tbl.indices(), 
tbl.columns(), tbl.colocationColumns());
+                SchemaValidationUtils.validateIndices(tbl.indices(), 
tbl.columns(), tbl.colocationColumns());
             } catch (IllegalArgumentException e) {
                 ctx.addIssue(new ValidationIssue(
                         ctx.currentKey(),
diff --git 
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/SchemaValidationUtils.java
 
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/SchemaValidationUtils.java
new file mode 100644
index 0000000000..4dd20017c6
--- /dev/null
+++ 
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/SchemaValidationUtils.java
@@ -0,0 +1,77 @@
+/*
+ * 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.schema.definition;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.ignite.schema.definition.ColumnDefinition;
+import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
+import org.apache.ignite.schema.definition.index.IndexColumnDefinition;
+import org.apache.ignite.schema.definition.index.IndexDefinition;
+
+/**
+ * Schema validation methods.
+ */
+public class SchemaValidationUtils {
+    /**
+     * Validate primary key.
+     *
+     * @param pkColNames Primary key columns.
+     * @param cols       Table columns.
+     */
+    public static void validatePrimaryKey(Set<String> pkColNames, final 
Map<String, ColumnDefinition> cols) {
+        pkColNames.stream()
+                .filter(pkCol -> cols.get(pkCol).nullable())
+                .findAny()
+                .ifPresent((pkCol) -> {
+                    throw new IllegalStateException("Primary key cannot 
contain nullable column [col=" + pkCol + "].");
+                });
+    }
+
+    /**
+     * Validate indices.
+     *
+     * @param indices Table indices.
+     * @param cols Table columns.
+     * @param colocationColNames Colocation columns names.
+     */
+    public static void validateIndices(
+            Collection<IndexDefinition> indices,
+            Collection<ColumnDefinition> cols,
+            List<String> colocationColNames) {
+        Set<String> colNames = 
cols.stream().map(ColumnDefinition::name).collect(Collectors.toSet());
+
+        for (IndexDefinition idx : indices) {
+            assert idx instanceof ColumnarIndexDefinition : "Only columnar 
indices are supported.";
+            // Note: E.g. functional index is not columnar index as it index 
an expression result only.
+
+            ColumnarIndexDefinition idx0 = (ColumnarIndexDefinition) idx;
+
+            if 
(!idx0.columns().stream().map(IndexColumnDefinition::name).allMatch(colNames::contains))
 {
+                throw new IllegalStateException("Index column must exist in 
the schema.");
+            }
+
+            if (idx0.unique() && 
!(idx0.columns().stream().map(IndexColumnDefinition::name).allMatch(colocationColNames::contains)))
 {
+                throw new IllegalStateException("Unique index must contains 
all colocation columns.");
+            }
+        }
+    }
+}
diff --git 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/AbstractIndexBuilder.java
 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/AbstractIndexBuilder.java
index 64bc9817b1..170825caaf 100644
--- 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/AbstractIndexBuilder.java
+++ 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/AbstractIndexBuilder.java
@@ -20,7 +20,6 @@ package org.apache.ignite.internal.schema.utils.builder;
 import java.util.Collections;
 import java.util.Map;
 import org.apache.ignite.internal.util.IgniteObjectName;
-import org.apache.ignite.schema.definition.builder.SchemaObjectBuilder;
 
 /**
  * Index base class.
diff --git 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilder.java
 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilder.java
index f3a809ff6b..62b3df40be 100644
--- 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilder.java
+++ 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilder.java
@@ -71,6 +71,20 @@ public interface SortedIndexDefinitionBuilder extends 
SchemaObjectBuilder {
          */
         SortedIndexColumnBuilder asc();
 
+        /**
+         * Sets nulls first sorting order.
+         *
+         * @return {@code this} for chaining.
+         */
+        SortedIndexColumnBuilder nullsFirst();
+
+        /**
+         * Sets nulls last sorting order.
+         *
+         * @return {@code this} for chaining.
+         */
+        SortedIndexColumnBuilder nullsLast();
+
         /**
          * Sets column name.
          *
diff --git 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilderImpl.java
 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilderImpl.java
index f384190b74..acc6496779 100644
--- 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilderImpl.java
+++ 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/SortedIndexDefinitionBuilderImpl.java
@@ -32,7 +32,7 @@ import 
org.apache.ignite.schema.definition.index.SortedIndexDefinition;
  * Sorted index builder.
  */
 class SortedIndexDefinitionBuilderImpl extends AbstractIndexBuilder implements 
SortedIndexDefinitionBuilder {
-    /** Index columns. */
+    /** Index columns ordered map. */
     protected final Map<String, SortedIndexColumnBuilderImpl> cols = new 
LinkedHashMap<>();
 
     /**
@@ -60,7 +60,6 @@ class SortedIndexDefinitionBuilderImpl extends 
AbstractIndexBuilder implements S
 
     /**
      * Add index column.
-     * TODO Documentation https://issues.apache.org/jira/browse/IGNITE-15859
      *
      * @param idxBuilder Index builder.
      */
@@ -82,7 +81,7 @@ class SortedIndexDefinitionBuilderImpl extends 
AbstractIndexBuilder implements S
      * Get index columns.
      */
     public List<SortedIndexColumnDefinition> columns() {
-        return cols.values().stream().map(c -> new 
SortedIndexColumnDefinitionImpl(c.name, c.asc)).collect(Collectors.toList());
+        return cols.values().stream().map(c -> new 
SortedIndexColumnDefinitionImpl(c.name, 
c.sortOrder())).collect(Collectors.toList());
     }
 
     /** {@inheritDoc} */
@@ -104,7 +103,10 @@ class SortedIndexDefinitionBuilderImpl extends 
AbstractIndexBuilder implements S
         protected String name;
 
         /** Index order flag. */
-        protected SortOrder asc = SortOrder.ASC_NULLS_FIRST;
+        private boolean asc = true;
+
+        /** Null first flag. */
+        private boolean nullsFirst = true;
 
         /**
          * Constructor.
@@ -118,7 +120,7 @@ class SortedIndexDefinitionBuilderImpl extends 
AbstractIndexBuilder implements S
         /** {@inheritDoc} */
         @Override
         public SortedIndexColumnBuilderImpl desc() {
-            asc = SortOrder.DESC_NULLS_LAST;
+            asc = false;
 
             return this;
         }
@@ -126,11 +128,32 @@ class SortedIndexDefinitionBuilderImpl extends 
AbstractIndexBuilder implements S
         /** {@inheritDoc} */
         @Override
         public SortedIndexColumnBuilderImpl asc() {
-            asc = SortOrder.ASC_NULLS_FIRST;
+            asc = true;
+
+            return this;
+        }
+
+        /** {@inheritDoc} */
+        @Override
+        public SortedIndexColumnBuilderImpl nullsFirst() {
+            nullsFirst = true;
 
             return this;
         }
 
+        /** {@inheritDoc} */
+        @Override
+        public SortedIndexColumnBuilderImpl nullsLast() {
+            nullsFirst = false;
+
+            return this;
+        }
+
+        protected SortOrder sortOrder() {
+            return asc ? (nullsFirst ? SortOrder.ASC_NULLS_FIRST : 
SortOrder.ASC_NULLS_LAST)
+                    : (nullsFirst ? SortOrder.DESC_NULLS_FIRST : 
SortOrder.DESC_NULLS_LAST);
+        }
+
         /** {@inheritDoc} */
         @Override
         public SortedIndexColumnBuilderImpl withName(String name) {
diff --git 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/TableDefinitionBuilderImpl.java
 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/TableDefinitionBuilderImpl.java
index d13af9a521..1f4f0b07ca 100644
--- 
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/TableDefinitionBuilderImpl.java
+++ 
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/utils/builder/TableDefinitionBuilderImpl.java
@@ -18,21 +18,17 @@
 package org.apache.ignite.internal.schema.utils.builder;
 
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
+import org.apache.ignite.internal.schema.definition.SchemaValidationUtils;
 import org.apache.ignite.internal.schema.definition.TableDefinitionImpl;
 import org.apache.ignite.internal.util.IgniteObjectName;
 import org.apache.ignite.schema.definition.ColumnDefinition;
 import org.apache.ignite.schema.definition.PrimaryKeyDefinition;
 import org.apache.ignite.schema.definition.TableDefinition;
-import org.apache.ignite.schema.definition.index.ColumnarIndexDefinition;
-import org.apache.ignite.schema.definition.index.IndexColumnDefinition;
 import org.apache.ignite.schema.definition.index.IndexDefinition;
 
 /**
@@ -58,7 +54,7 @@ class TableDefinitionBuilderImpl implements 
TableDefinitionBuilder {
      * Constructor.
      *
      * @param schemaName Schema name.
-     * @param tableName  Table name.
+     * @param tableName Table name.
      */
     public TableDefinitionBuilderImpl(String schemaName, String tableName) {
         this.schemaName = IgniteObjectName.parse(schemaName);
@@ -124,8 +120,8 @@ class TableDefinitionBuilderImpl implements 
TableDefinitionBuilder {
         assert primaryKeyDefinition != null : "Primary key index must be 
configured.";
         assert columns.size() > primaryKeyDefinition.columns().size() : "Key 
or/and value columns must be defined.";
 
-        validatePrimaryKey(primaryKeyDefinition.columns(), columns);
-        validateIndices(indices.values(), columns.values(), 
primaryKeyDefinition.colocationColumns());
+        
SchemaValidationUtils.validatePrimaryKey(primaryKeyDefinition.columns(), 
columns);
+        SchemaValidationUtils.validateIndices(indices.values(), 
columns.values(), primaryKeyDefinition.colocationColumns());
 
         return new TableDefinitionImpl(
                 schemaName,
@@ -135,48 +131,4 @@ class TableDefinitionBuilderImpl implements 
TableDefinitionBuilder {
                 Collections.unmodifiableMap(indices)
         );
     }
-
-    /**
-     * Validate primary key.
-     *
-     * @param pkColNames Primary key columns.
-     * @param cols       Table columns.
-     */
-    private static void validatePrimaryKey(Set<String> pkColNames, final 
Map<String, ColumnDefinition> cols) {
-        pkColNames.stream()
-                .filter(pkCol -> cols.get(pkCol).nullable())
-                .findAny()
-                .ifPresent((pkCol) -> {
-                    throw new IllegalStateException("Primary key cannot 
contain nullable column [col=" + pkCol + "].");
-                });
-    }
-
-    /**
-     * Validate indices.
-     *
-     * @param indices Table indices.
-     * @param cols Table columns.
-     * @param colocationColNames Colocation columns names.
-     */
-    public static void validateIndices(
-            Collection<IndexDefinition> indices,
-            Collection<ColumnDefinition> cols,
-            List<String> colocationColNames) {
-        Set<String> colNames = 
cols.stream().map(ColumnDefinition::name).collect(Collectors.toSet());
-
-        for (IndexDefinition idx : indices) {
-            assert idx instanceof ColumnarIndexDefinition : "Only columnar 
indices are supported.";
-            // Note: E.g. functional index is not columnar index as it index 
an expression result only.
-
-            ColumnarIndexDefinition idx0 = (ColumnarIndexDefinition) idx;
-
-            if 
(!idx0.columns().stream().map(IndexColumnDefinition::name).allMatch(colNames::contains))
 {
-                throw new IllegalStateException("Index column must exist in 
the schema.");
-            }
-
-            if (idx0.unique() && 
!(idx0.columns().stream().map(IndexColumnDefinition::name).allMatch(colocationColNames::contains)))
 {
-                throw new IllegalStateException("Unique index must contains 
all colocation columns.");
-            }
-        }
-    }
 }
diff --git 
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
 
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
index 1f8cbd52bd..05eb1f4dbc 100644
--- 
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
+++ 
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
@@ -36,7 +36,9 @@ import 
org.apache.ignite.configuration.schemas.table.ConstantValueDefaultChange;
 import org.apache.ignite.configuration.schemas.table.FunctionCallDefaultChange;
 import org.apache.ignite.configuration.schemas.table.NullValueDefaultChange;
 import org.apache.ignite.configuration.schemas.table.PrimaryKeyView;
+import org.apache.ignite.configuration.schemas.table.SortedIndexChange;
 import org.apache.ignite.configuration.schemas.table.TableChange;
+import org.apache.ignite.configuration.schemas.table.TableIndexChange;
 import org.apache.ignite.internal.index.IndexManager;
 import org.apache.ignite.internal.schema.definition.TableDefinitionImpl;
 import 
org.apache.ignite.internal.sql.engine.prepare.ddl.AbstractTableDdlCommand;
@@ -60,12 +62,10 @@ 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.IgniteInternalException;
 import org.apache.ignite.lang.IgniteStringFormatter;
 import org.apache.ignite.lang.TableAlreadyExistsException;
 import org.apache.ignite.lang.TableNotFoundException;
-import org.apache.ignite.schema.SchemaBuilders;
-import 
org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder;
-import 
org.apache.ignite.schema.definition.builder.SortedIndexDefinitionBuilder.SortedIndexColumnBuilder;
 
 /** DDL commands handler. */
 public class DdlCommandHandler {
@@ -236,26 +236,20 @@ public class DdlCommandHandler {
 
     /** Handles create index command. */
     private boolean handleCreateIndex(CreateIndexCommand cmd) {
-        // Only sorted idx for now.
-        //TODO: https://issues.apache.org/jira/browse/IGNITE-17563 Pass null 
ordering for columns.
-        SortedIndexDefinitionBuilder idx = 
SchemaBuilders.sortedIndex(cmd.indexName());
-
-        for (Pair<String, Boolean> idxInfo : cmd.columns()) {
-            SortedIndexColumnBuilder idx0 = 
idx.addIndexColumn(idxInfo.getFirst());
-
-            if (idxInfo.getSecond()) {
-                idx0.desc();
+        Consumer<TableIndexChange> indexChanger = tableIndexChange -> {
+            if (tableIndexChange instanceof SortedIndexChange) {
+                createSortedIndexInternal(cmd, 
tableIndexChange.convert(SortedIndexChange.class));
             }
 
-            idx0.done();
-        }
+            throw new IgniteInternalException("Only sorted index type is 
supported.");
+        };
 
         return indexManager.createIndex(
                 cmd.schemaName(),
                 cmd.indexName(),
                 cmd.tableName(),
                 !cmd.ifIndexNotExists(),
-                tableIndexChange -> convert(idx.build(), tableIndexChange));
+                indexChanger);
     }
 
     /** Handles drop index command. */
@@ -263,6 +257,23 @@ public class DdlCommandHandler {
         return indexManager.dropIndex(cmd.schemaName(), cmd.indexName(), 
!cmd.ifNotExists());
     }
 
+
+    /**
+     * Creates sorted index.
+     *
+     * @param cmd Create index command.
+     * @param tableIndexChange Index configuration changer.
+     */
+    private void createSortedIndexInternal(CreateIndexCommand cmd, 
SortedIndexChange tableIndexChange) {
+        tableIndexChange
+                .convert(SortedIndexChange.class)
+                .changeColumns(colsInit -> {
+                    for (Pair<String, Boolean> col : cmd.columns()) {
+                        colsInit.create(col.getFirst(), colInit -> 
colInit.changeAsc(col.getSecond()));
+                    }
+                });
+    }
+
     /**
      * Adds a column according to the column definition.
      *

Reply via email to