korlov42 commented on code in PR #1142: URL: https://github.com/apache/ignite-3/pull/1142#discussion_r985824411
########## modules/core/src/main/java/org/apache/ignite/internal/util/IgniteNameUtils.java: ########## @@ -0,0 +1,169 @@ +/* + * 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.util; + +import static org.apache.ignite.lang.ErrorGroups.Common.ILLEGAL_ARGUMENT_ERR; + +import org.apache.ignite.lang.IgniteInternalException; +import org.jetbrains.annotations.Nullable; + +/** + * Utility methods used for cluster's named objects: schemas, tables, columns, indexes, etc. + */ +public final class IgniteNameUtils { + /** No instance methods. */ + private IgniteNameUtils() { + } + + /** + * Parse simple name: unquote name or cast to upper case not-quoted name. + * + * @param name String to parse object name. + * @return Unquoted name or name is cast to upper case. "tbl0" -> "TBL0", "\"Tbl0\"" -> "Tbl0". + */ + public static String parseSimpleName(String name) { + if (StringUtils.nullOrEmpty(name)) { + return name; + } + + var tokenizer = new Tokenizer(name); + + String parsedName = tokenizer.nextToken(); + + if (tokenizer.hasNext()) { + throw new IgniteInternalException(ILLEGAL_ARGUMENT_ERR, "Fully qualified name is not expected [name=" + name + "]"); + } + + return parsedName; + } + + public static String canonicalName(String schemaName, String objectName) { Review Comment: Good catch! I'm wondering why does checkstyle miss this one 🤔 ########## modules/core/src/main/java/org/apache/ignite/internal/util/IgniteNameUtils.java: ########## @@ -0,0 +1,169 @@ +/* + * 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.util; + +import static org.apache.ignite.lang.ErrorGroups.Common.ILLEGAL_ARGUMENT_ERR; + +import org.apache.ignite.lang.IgniteInternalException; +import org.jetbrains.annotations.Nullable; + +/** + * Utility methods used for cluster's named objects: schemas, tables, columns, indexes, etc. + */ +public final class IgniteNameUtils { + /** No instance methods. */ + private IgniteNameUtils() { + } + + /** + * Parse simple name: unquote name or cast to upper case not-quoted name. + * + * @param name String to parse object name. + * @return Unquoted name or name is cast to upper case. "tbl0" -> "TBL0", "\"Tbl0\"" -> "Tbl0". + */ + public static String parseSimpleName(String name) { + if (StringUtils.nullOrEmpty(name)) { + return name; + } + + var tokenizer = new Tokenizer(name); + + String parsedName = tokenizer.nextToken(); + + if (tokenizer.hasNext()) { + throw new IgniteInternalException(ILLEGAL_ARGUMENT_ERR, "Fully qualified name is not expected [name=" + name + "]"); + } + + return parsedName; + } + + public static String canonicalName(String schemaName, String objectName) { + return quote(schemaName) + '.' + quote(objectName); + } + + /** + * Wraps the given name with double quotes, e.g. "myColumn" -> "\"myColumn\"" + * + * @param name Object name. + * @return Quoted object name. + */ + public static String quote(String name) { + return "\"" + name + "\""; + } + + /** + * Identifier chain tokenizer. + * + * <p>Splits provided identifier chain (complex identifier like PUBLIC.MY_TABLE) into its component parts. + * + * <p>This tokenizer is not SQL compliant, but it is ok since it used to retrieve an object only. The sole purpose of this tokenizer + * is to split the chain into parts by a dot considering the quotation. + */ + private static class Tokenizer { + private int currentPosition; + private final String source; + + /** + * Creates a tokenizer for given string source. + * + * @param source Source string to split. + */ + public Tokenizer(String source) { + this.source = source; + } + + /** Returns {@code true} if at least one token is available. */ + public boolean hasNext() { + return currentPosition < source.length(); + } + + /** Returns next token. */ + public @Nullable String nextToken() { + if (!hasNext()) { + return null; + } + + boolean quoted = source.charAt(currentPosition) == '"'; + + if (quoted) { + currentPosition++; + } + + int start = currentPosition; + StringBuilder sb = new StringBuilder(); + + for (; currentPosition < source.length(); currentPosition++) { + if (currentChar() == '"') { + if (!quoted) { + throwMalformedNameException(); + } + + if (hasNextChar() && nextChar() == '"') { // quote is escaped + sb.append(source, start, currentPosition + 1); + + currentPosition += 2; + start = currentPosition; + + continue; + } else if (!hasNextChar() || nextChar() == '.') { + // looks like we just found a closing quote + sb.append(source, start, currentPosition); + + currentPosition += 2; + + return sb.toString(); + } + + throwMalformedNameException(); + } else if (!quoted && (currentChar() == '.' || currentChar() == ' ')) { + sb.append(source, start, currentPosition); + + currentPosition++; + + return sb.toString().toUpperCase(); + } + } + + if (quoted) { + // seems like there is no closing quote + throwMalformedNameException(); + } + + return source.substring(start).toUpperCase(); + } + + private char currentChar() { + return source.charAt(currentPosition); + } + + private boolean hasNextChar() { + return currentPosition + 1 < source.length(); + } + + private char nextChar() { + return source.charAt(currentPosition + 1); + } + + private void throwMalformedNameException() { Review Comment: Actually, we already have a bunch of such tests. Please take a look at `org.apache.ignite.internal.util.IgniteNameUtilsTest#malformedSimpleNames` ########## modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java: ########## @@ -149,16 +139,16 @@ public CompletableFuture<Boolean> createIndexAsync( tablesCfg.indexes().change(indexListChange -> { idxExist.set(false); - if (indexListChange.get(canonicalIndexName) != null) { + if (indexListChange.get(indexName) != null) { Review Comment: Currently we have only one schema: PUBLIC. So, we will deal with this we we add custom schema support ########## modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteInMemoryNodeRestartTest.java: ########## @@ -325,26 +312,14 @@ private static void checkTableWithData(Ignite ignite, String name) { * @param partitions Partitions count. */ private static void createTableWithData(Ignite ignite, String name, int replicas, int partitions) { - TableDefinition scmTbl1 = SchemaBuilders.tableBuilder("PUBLIC", name).columns( - SchemaBuilders.column("id", ColumnType.INT32).build(), - SchemaBuilders.column("name", ColumnType.string()).asNullable(true).build() - ).withPrimaryKey( - SchemaBuilders.primaryKey() - .withColumns("id") - .build() - ).build(); - - Table table = ignite.tables().createTable( - scmTbl1.canonicalName(), - tbl -> convert(scmTbl1, tbl).changeReplicas(replicas).changePartitions(partitions) - .changeDataStorage(dsc -> dsc.convert(VolatilePageMemoryDataStorageChange.class)) - ); + try (Session session = ignite.sql().createSession()) { + session.execute(null, "CREATE TABLE " + name + + "(id INT PRIMARY KEY, name VARCHAR) WITH replicas=" + replicas + ", partitions=" + partitions); - for (int i = 0; i < 100; i++) { - Tuple key = Tuple.create().set("id", i); - Tuple val = Tuple.create().set("name", VALUE_PRODUCER.apply(i)); - - table.keyValueView().put(null, key, val); + for (int i = 0; i < 100; i++) { + session.execute(null, "INSERT INTO " + name + "(id, name) VALUES (?, ?)", Review Comment: I believe it's good enough for a hundred of rows :) ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java: ########## @@ -377,44 +355,44 @@ private void convertColumnDefinition(ColumnDefinition definition, ColumnChange c /** * Drops a column(s) exceptional behavior depends on {@code colExist} flag. * - * @param fullName Table with schema name. + * @param tableName Table name. * @param colNames Columns definitions. - * @param ignoreColumnExistance Flag indicates exceptionally behavior in case of already existing column. + * @param ignoreColumnExistence Flag indicates exceptionally behavior in case of already existing column. * @return {@code true} if the full columns set is applied successfully. Otherwise, returns {@code false}. */ - private CompletableFuture<Boolean> dropColumnInternal(String fullName, Set<String> colNames, boolean ignoreColumnExistance) { + private CompletableFuture<Boolean> dropColumnInternal(String tableName, Set<String> colNames, boolean ignoreColumnExistence) { AtomicBoolean ret = new AtomicBoolean(true); return tableManager.alterTableAsync( - fullName, - chng -> chng.changeColumns(cols -> { - PrimaryKeyView priKey = chng.primaryKey(); - - Map<String, String> colNamesToOrders = columnOrdersToNames(chng.columns()); + tableName, + chng -> chng.changeColumns(cols -> { + PrimaryKeyView priKey = chng.primaryKey(); - Set<String> colNames0 = new HashSet<>(); + Map<String, String> colNamesToOrders = columnOrdersToNames(chng.columns()); - Set<String> primaryCols = Set.of(priKey.columns()); + Set<String> colNames0 = new HashSet<>(); - for (String colName : colNames) { - if (!colNamesToOrders.containsKey(colName)) { - ret.set(false); + Set<String> primaryCols = Set.of(priKey.columns()); - if (!ignoreColumnExistance) { - throw new ColumnNotFoundException(colName, fullName); - } - } else { - colNames0.add(colName); - } + for (String colName : colNames) { + if (!colNamesToOrders.containsKey(colName)) { + ret.set(false); - if (primaryCols.contains(colName)) { - throw new SqlException(DEL_PK_COMUMN_CONSTRAINT_ERR, IgniteStringFormatter - .format("Can`t delete column, belongs to primary key: [name={}]", colName)); - } + if (!ignoreColumnExistence) { + throw new ColumnNotFoundException(colName); Review Comment: Just to make it similar to Client API. Relation can be acquired from context. ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java: ########## @@ -377,44 +355,44 @@ private void convertColumnDefinition(ColumnDefinition definition, ColumnChange c /** * Drops a column(s) exceptional behavior depends on {@code colExist} flag. * - * @param fullName Table with schema name. + * @param tableName Table name. * @param colNames Columns definitions. - * @param ignoreColumnExistance Flag indicates exceptionally behavior in case of already existing column. + * @param ignoreColumnExistence Flag indicates exceptionally behavior in case of already existing column. * @return {@code true} if the full columns set is applied successfully. Otherwise, returns {@code false}. */ - private CompletableFuture<Boolean> dropColumnInternal(String fullName, Set<String> colNames, boolean ignoreColumnExistance) { + private CompletableFuture<Boolean> dropColumnInternal(String tableName, Set<String> colNames, boolean ignoreColumnExistence) { AtomicBoolean ret = new AtomicBoolean(true); return tableManager.alterTableAsync( - fullName, - chng -> chng.changeColumns(cols -> { - PrimaryKeyView priKey = chng.primaryKey(); - - Map<String, String> colNamesToOrders = columnOrdersToNames(chng.columns()); + tableName, + chng -> chng.changeColumns(cols -> { + PrimaryKeyView priKey = chng.primaryKey(); - Set<String> colNames0 = new HashSet<>(); + Map<String, String> colNamesToOrders = columnOrdersToNames(chng.columns()); - Set<String> primaryCols = Set.of(priKey.columns()); + Set<String> colNames0 = new HashSet<>(); - for (String colName : colNames) { - if (!colNamesToOrders.containsKey(colName)) { - ret.set(false); + Set<String> primaryCols = Set.of(priKey.columns()); - if (!ignoreColumnExistance) { - throw new ColumnNotFoundException(colName, fullName); - } - } else { - colNames0.add(colName); - } + for (String colName : colNames) { + if (!colNamesToOrders.containsKey(colName)) { + ret.set(false); - if (primaryCols.contains(colName)) { - throw new SqlException(DEL_PK_COMUMN_CONSTRAINT_ERR, IgniteStringFormatter - .format("Can`t delete column, belongs to primary key: [name={}]", colName)); - } + if (!ignoreColumnExistence) { + throw new ColumnNotFoundException(colName); Review Comment: Just to make it similar to Client API. Relation can be acquired from context. Well, could revert this part, if you insist -- 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]
