zstan commented on code in PR #1142:
URL: https://github.com/apache/ignite-3/pull/1142#discussion_r985657229
##########
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:
In such a case we obtain index uniqueness through all schemes, is it ok ? PG
9.6 accepts equal idx names through different schemes :
`create table test (a int);
create schema pub;
create table pub.test (a int);
create index i1 on test(a);
create index i1 on pub.test(a);` - no err found.
##########
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:
Why don`t we need additional "table name" info here ? i think it helpful,
check with PG:
`column "b" of relation "test" does not exist`
##########
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:
probably we need at least one small test for such exception ?:)
##########
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:
probably using of "executeBatch" would be more accurate in such a case ?
##########
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:
javadoc`me please )
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##########
@@ -1025,25 +1019,14 @@ private static void createTableWithData(Ignite ignite,
String name, int replicas
* @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)
- );
+ 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:
probably - executeBatch ?
--
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]
