You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tkalkirill (via GitHub)" <gi...@apache.org> on 2023/06/23 07:12:16 UTC

[GitHub] [ignite-3] tkalkirill opened a new pull request, #2243: IGNITE-19799 Refactoring classes associated with the catalog

tkalkirill opened a new pull request, #2243:
URL: https://github.com/apache/ignite-3/pull/2243

   https://issues.apache.org/jira/browse/IGNITE-19799


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239685753


##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -145,7 +146,7 @@ public static <T> CompletableFutureMatcher<T> willBe(Matcher<T> matcher) {
      * @param <T> value type
      * @return matcher
      */
-    public static <T> CompletableFutureMatcher<T> willBe(T value) {
+    public static <T> CompletableFutureMatcher<T> willBe(@Nullable T value) {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239689410


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] rpuch commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239645143


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {

Review Comment:
   Let's rename the method to `validateCreateTableParams()`, otherwise it is a bit difficult to read the code where the method is invoked



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+
+        if (params.collations().size() != params.columns().size()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Columns collations doesn't match number of columns."
+            );
+        }
+    }
+
+    private static void validateParams(CreateZoneParams params) {
+        if (params.dataNodesAutoAdjust() != INFINITE_TIMER_VALUE
+                && (params.dataNodesAutoAdjustScaleUp() != INFINITE_TIMER_VALUE
+                || params.dataNodesAutoAdjustScaleDown() != INFINITE_TIMER_VALUE)
+        ) {
+            throw new IgniteInternalException(
+                    DistributionZones.ZONE_DEFINITION_ERR,
+                    "Not compatible parameters [dataNodesAutoAdjust={}, dataNodesAutoAdjustScaleUp={}, dataNodesAutoAdjustScaleDown={}]",
+                    params.dataNodesAutoAdjust(), params.dataNodesAutoAdjustScaleUp(), params.dataNodesAutoAdjustScaleDown()
+            );
+        }
+    }
+
+    private static void validateIndexColumns(List<String> indexColumns, CatalogTableDescriptor table) {

Review Comment:
   Let's rename the method



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);

Review Comment:
   Why do we throw an NPE when there is no such schema, but a specific exception when there is no such zone/table?



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {

Review Comment:
   Let's rename the method



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {

Review Comment:
   Let's rename the method



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {

Review Comment:
   validateAlterTableDropColumnParams



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+
+        if (params.collations().size() != params.columns().size()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Columns collations doesn't match number of columns."
+            );
+        }
+    }
+
+    private static void validateParams(CreateZoneParams params) {
+        if (params.dataNodesAutoAdjust() != INFINITE_TIMER_VALUE
+                && (params.dataNodesAutoAdjustScaleUp() != INFINITE_TIMER_VALUE
+                || params.dataNodesAutoAdjustScaleDown() != INFINITE_TIMER_VALUE)
+        ) {
+            throw new IgniteInternalException(
+                    DistributionZones.ZONE_DEFINITION_ERR,
+                    "Not compatible parameters [dataNodesAutoAdjust={}, dataNodesAutoAdjustScaleUp={}, dataNodesAutoAdjustScaleDown={}]",
+                    params.dataNodesAutoAdjust(), params.dataNodesAutoAdjustScaleUp(), params.dataNodesAutoAdjustScaleDown()
+            );
+        }
+    }
+
+    private static void validateIndexColumns(List<String> indexColumns, CatalogTableDescriptor table) {
+        if (indexColumns.isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "No index columns was specified."
+            );
+        }
+
+        Predicate<String> duplicateValidator = Predicate.not(new HashSet<>()::add);
+
+        for (String columnName : indexColumns) {
+            CatalogTableColumnDescriptor columnDescriptor = table.columnDescriptor(columnName);
+
+            if (columnDescriptor == null) {
+                throw new ColumnNotFoundException(columnName);
+            } else if (duplicateValidator.test(columnName)) {
+                throw new IgniteInternalException(
+                        ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                        "Can't create index on duplicate columns: {}",
+                        String.join(", ", indexColumns)
+                );
+            }
+        }
+    }
+
+    private static void validateAlterTableColumn(

Review Comment:
   Let's rename the method (and the following ones)



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+
+        if (params.collations().size() != params.columns().size()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Columns collations doesn't match number of columns."
+            );
+        }
+    }
+
+    private static void validateParams(CreateZoneParams params) {

Review Comment:
   Let's rename the method



##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/matchers/CompletableFutureMatcher.java:
##########
@@ -145,7 +146,7 @@ public static <T> CompletableFutureMatcher<T> willBe(Matcher<T> matcher) {
      * @param <T> value type
      * @return matcher
      */
-    public static <T> CompletableFutureMatcher<T> willBe(T value) {
+    public static <T> CompletableFutureMatcher<T> willBe(@Nullable T value) {

Review Comment:
   willBe(null) should never be used; instead, willBe(nullValue()) is preferred, so this annotation is not needed here. Please don't replace willBe(null) in the code, I've already done this in my PR that will soon be merged



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);

Review Comment:
   Let's also check that table name is not null



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill merged pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill merged PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239527691


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractIndexCommandParams.java:
##########
@@ -41,6 +47,20 @@ public String schemaName() {
         return schema;
     }
 
+    /**
+     * Returns table name.
+     */
+    public String tableName() {
+        return tableName;
+    }
+
+    /**
+     * Returns {@code true} if index is unique, {@code false} otherwise.
+     */
+    public boolean isUnique() {

Review Comment:
   agree, I'll fix it without a prefix.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239687735


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+
+        if (params.collations().size() != params.columns().size()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Columns collations doesn't match number of columns."
+            );
+        }
+    }
+
+    private static void validateParams(CreateZoneParams params) {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239684236


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractIndexCommandParams.java:
##########
@@ -41,6 +47,20 @@ public String schemaName() {
         return schema;
     }
 
+    /**
+     * Returns table name.
+     */
+    public String tableName() {
+        return tableName;
+    }
+
+    /**
+     * Returns {@code true} if index is unique, {@code false} otherwise.
+     */
+    public boolean isUnique() {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239687539


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {

Review Comment:
   fix it



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {
+        for (String columnName : params.columns()) {
+            if (table.column(columnName) == null) {
+                throw new ColumnNotFoundException(columnName);
+            }
+
+            if (table.isPrimaryKeyColumn(columnName)) {
+                throw new SqlException(
+                        Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                        "Can't drop primary key column: [column={}]",
+                        columnName
+                );
+            }
+        }
+
+        Arrays.stream(schema.indexes())
+                .filter(index -> index.tableId() == table.id())
+                .forEach(index -> params.columns().stream()
+                        .filter(index::hasColumn)
+                        .findAny()
+                        .ifPresent(columnName -> {
+                            throw new SqlException(
+                                    Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
+                                    "Can't drop indexed column: [columnName={}, indexName={}]",
+                                    columnName, index.name()
+                            );
+                        }));
+    }
+
+    private static void validateParams(CreateHashIndexParams params, CatalogTableDescriptor table) {
+        validateIndexColumns(params.columns(), table);
+    }
+
+    private static void validateParams(CreateSortedIndexParams params, CatalogTableDescriptor table) {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239690756


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239686857


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {
+        params.columns().stream()
+                .map(ColumnParams::name)
+                .filter(Predicate.not(new HashSet<>()::add))
+                .findAny()
+                .ifPresent(columnName -> {
+                    throw new IgniteInternalException(
+                            ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                            "Can't create table with duplicate columns: {}",
+                            params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+                    );
+                });
+
+        if (params.primaryKeyColumns().isEmpty()) {
+            throw new IgniteInternalException(
+                    ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+                    "Missing primary key columns"
+            );
+        }
+    }
+
+    private static void validateParams(AlterTableDropColumnParams params, CatalogSchemaDescriptor schema, CatalogTableDescriptor table) {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239686536


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -863,4 +654,197 @@ private Catalog applyUpdateFinal(Catalog catalog, VersionedUpdate update) {
                 catalog.schemas()
         );
     }
+
+    private static CatalogSchemaDescriptor getSchema(Catalog catalog, @Nullable String schemaName) {
+        schemaName = Objects.requireNonNullElse(schemaName, DEFAULT_SCHEMA_NAME);
+
+        return Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName);
+    }
+
+    private static CatalogTableDescriptor getTable(CatalogSchemaDescriptor schema, @Nullable String tableName) {
+        CatalogTableDescriptor table = schema.table(tableName);
+
+        if (table == null) {
+            throw new TableNotFoundException(schema.name(), tableName);
+        }
+
+        return table;
+    }
+
+    private static CatalogZoneDescriptor getZone(Catalog catalog, String zoneName) {
+        zoneName = Objects.requireNonNull(zoneName, "zoneName");
+
+        CatalogZoneDescriptor zone = catalog.zone(zoneName);
+
+        if (zone == null) {
+            throw new DistributionZoneNotFoundException(zoneName);
+        }
+
+        return zone;
+    }
+
+    private static CatalogTableColumnDescriptor findTableColumn(CatalogTableDescriptor table, String columnName) {
+        return table.columns().stream()
+                .filter(desc -> desc.name().equals(columnName))
+                .findFirst()
+                .orElseThrow(() -> new ColumnNotFoundException(columnName));
+    }
+
+    private static CatalogTableColumnDescriptor createNewTableColumn(AlterColumnParams params, CatalogTableColumnDescriptor origin) {
+        return new CatalogTableColumnDescriptor(
+                origin.name(),
+                Objects.requireNonNullElse(params.type(), origin.type()),
+                !Objects.requireNonNullElse(params.notNull(), !origin.nullable()),
+                Objects.requireNonNullElse(params.precision(), origin.precision()),
+                Objects.requireNonNullElse(params.scale(), origin.scale()),
+                Objects.requireNonNullElse(params.length(), origin.length()),
+                Objects.requireNonNullElse(params.defaultValue(origin.type()), origin.defaultValue())
+        );
+    }
+
+    private static void validateParams(CreateTableParams params) {

Review Comment:
   fix it



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239497171


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -137,66 +137,61 @@ public CatalogServiceImpl(UpdateLog updateLog, HybridClock clock, long delayDura
         this.delayDurationMs = delayDurationMs;
     }
 
-    /** {@inheritDoc} */

Review Comment:
   They are useless, if you open the documentation, then the parent is automatically loaded when reading from the development environment.



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] korlov42 commented on a diff in pull request #2243: IGNITE-19799 Refactoring classes associated with the catalog

Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #2243:
URL: https://github.com/apache/ignite-3/pull/2243#discussion_r1239486944


##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractIndexCommandParams.java:
##########
@@ -41,6 +47,20 @@ public String schemaName() {
         return schema;
     }
 
+    /**
+     * Returns table name.
+     */
+    public String tableName() {
+        return tableName;
+    }
+
+    /**
+     * Returns {@code true} if index is unique, {@code false} otherwise.
+     */
+    public boolean isUnique() {

Review Comment:
   we either need to add prefix to every getter or don't use it at all within *Params classes



##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java:
##########
@@ -137,66 +137,61 @@ public CatalogServiceImpl(UpdateLog updateLog, HybridClock clock, long delayDura
         this.delayDurationMs = delayDurationMs;
     }
 
-    /** {@inheritDoc} */

Review Comment:
   what is wrong with this javadoc (and rest of the similar)?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org