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

[PR] IGNITE-24021 [ignite-3]

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

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


-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {
+            int codePoint = identifier.codePointAt(pos);
+
+            if (!identifierExtend(codePoint) && !Character.isUpperCase(codePoint)) {

Review Comment:
   ```suggestion
               if (!identifierExtend(codePoint) || !Character.isUpperCase(codePoint)) {
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {
+            int codePoint = identifier.codePointAt(pos);
+
+            if (!identifierExtend(codePoint) && !Character.isUpperCase(codePoint)) {

Review Comment:
   ```suggestion
               if (!(identifierStart(codePoint) || identifierExtend(codePoint)) || !Character.isUpperCase(codePoint)) {
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {

Review Comment:
   ```suggestion
           for (int pos = 1; pos < identifier.length(); pos++) {
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with
   ```
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks confusing, considering that we are passing the name of the scheme



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -213,29 +222,44 @@ public boolean hasNext() {
                         // looks like we just found a closing quote
                         sb.append(source, start, currentPosition);
 
+                        foundDot = hasNextChar();
                         currentPosition += 2;
 
                         return sb.toString();
                     }
 
-                    throwMalformedNameException();
-                } else if (!quoted && (currentChar() == '.' || currentChar() == ' ')) {
+                    throwMalformedIdentifierException();
+                } else if (c == '.') {
+                    if (quoted) {
+                        continue;
+                    }
+
                     sb.append(source, start, currentPosition);
 
                     currentPosition++;
+                    foundDot = true;
 
                     return sb.toString().toUpperCase();
+                } else if (!quoted
+                        && !identifierStart(source.codePointAt(currentPosition))
+                        && !identifierExtend(source.codePointAt(currentPosition))

Review Comment:
   No, it doesn't.



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {

Review Comment:
   ```suggestion
           if (!identifierStart(identifier.codePointAt(0)) || !Character.isUpperCase(identifier.codePointAt(0))) {
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -213,29 +222,44 @@ public boolean hasNext() {
                         // looks like we just found a closing quote
                         sb.append(source, start, currentPosition);
 
+                        foundDot = hasNextChar();
                         currentPosition += 2;
 
                         return sb.toString();
                     }
 
-                    throwMalformedNameException();
-                } else if (!quoted && (currentChar() == '.' || currentChar() == ' ')) {
+                    throwMalformedIdentifierException();
+                } else if (c == '.') {
+                    if (quoted) {
+                        continue;
+                    }
+
                     sb.append(source, start, currentPosition);
 
                     currentPosition++;
+                    foundDot = true;
 
                     return sb.toString().toUpperCase();
+                } else if (!quoted
+                        && !identifierStart(source.codePointAt(currentPosition))
+                        && !identifierExtend(source.codePointAt(currentPosition))

Review Comment:
   Including alphabetic characters (`indentifierStart`) into `identifierExtend` will make this code simpler.
   



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();

Review Comment:
   ```suggestion
           List<String> identifiers = new ArrayList<>(2);
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {
+            int codePoint = identifier.codePointAt(pos);
+
+            if (!identifierExtend(codePoint) && !Character.isUpperCase(codePoint)) {

Review Comment:
   ```suggestion
               if (!(identifierStart || identifierExtend(codePoint)) || !Character.isUpperCase(codePoint)) {
   ```



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with another TODO
   ```
   // TODO: https://issues.apache.org/jira/browse/IGNITE-24021
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks confusing, considering that we are passing the name of the schema



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with another TODO
   ```
   // TODO: https://issues.apache.org/jira/browse/IGNITE-24021
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks weird and a bit confusing, considering that we are passing the name of the schema, but getting object name (not schema name)



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/table/QualifiedName.java:
##########
@@ -107,15 +108,34 @@ public static QualifiedName fromSimple(String simpleName) {
      * @return Qualified name.
      */
     public static QualifiedName of(@Nullable String schemaName, String objectName) {
-        String schemaIdentifier = schemaName == null ? DEFAULT_SCHEMA_NAME : parseIdentifier(schemaName);
-        String objectIdentifier = parseIdentifier(objectName);
+        if (schemaName == null) {
+            schemaName = DEFAULT_SCHEMA_NAME;
+        }
 
-        verifySchemaIdentifier(schemaIdentifier);
-        verifyObjectIdentifier(objectIdentifier);
+        verifyObjectIdentifier(objectName);
+        verifySchemaIdentifier(schemaName);
+
+        String schemaIdentifier = parseSimpleName(schemaName);
+        String objectIdentifier = parseSimpleName(objectName);
 
         return new QualifiedName(schemaIdentifier, objectIdentifier);
     }
 
+    /**
+     * Return QualifiedName from a given schema and table normalized names.
+     *
+     * @param schemaName Normalized schema name or {@code null} for default schema.
+     * @param objectName Normalized object name.
+     * @return Qualified name.
+     */
+    public static QualifiedName fromNormalized(@Nullable String schemaName, String objectName) {
+        verifySchemaIdentifier(schemaName);
+        verifyObjectIdentifier(objectName);
+
+        return new QualifiedName(schemaName == null ? DEFAULT_SCHEMA_NAME : schemaName, objectName);
+    }
+
+

Review Comment:
   I think this empty line is unnecessary



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/test/java/org/apache/ignite/table/QualifiedNameTest.java:
##########
@@ -273,9 +270,4 @@ public void malformedCanonicalNames(String source) {
                 containsString("Malformed identifier [identifier=" + source)
         )));
     }
-
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 Move to IgniteNameUtils
-    private static String canonicalName(String schemaName, String objectName) {
-        return schemaName == null ? quoteIfNeeded(objectName) : quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);

Review Comment:
   There is no reason to allow `null` for `schemaName`. You can use `quoteIfNeeded` instead.
   
   Alternatively we can change method signature to 
   ```
   canonicalName(String ... names)
   ```
   and replace quoteIfNeeded(name) with canonicalName(name) everywhere.... thus making `quoteIfNeeded` private :thinking: 



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -213,29 +222,44 @@ public boolean hasNext() {
                         // looks like we just found a closing quote
                         sb.append(source, start, currentPosition);
 
+                        foundDot = hasNextChar();
                         currentPosition += 2;
 
                         return sb.toString();
                     }
 
-                    throwMalformedNameException();
-                } else if (!quoted && (currentChar() == '.' || currentChar() == ' ')) {
+                    throwMalformedIdentifierException();
+                } else if (c == '.') {
+                    if (quoted) {
+                        continue;
+                    }
+
                     sb.append(source, start, currentPosition);
 
                     currentPosition++;
+                    foundDot = true;
 
                     return sb.toString().toUpperCase();
+                } else if (!quoted
+                        && !identifierStart(source.codePointAt(currentPosition))
+                        && !identifierExtend(source.codePointAt(currentPosition))

Review Comment:
   Does `identifierExtend` characters includes `identifierStart` characters?
   If so, there is no need to check if characters is `identifierStart`.



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/table/QualifiedName.java:
##########
@@ -107,15 +108,34 @@ public static QualifiedName fromSimple(String simpleName) {
      * @return Qualified name.
      */
     public static QualifiedName of(@Nullable String schemaName, String objectName) {
-        String schemaIdentifier = schemaName == null ? DEFAULT_SCHEMA_NAME : parseIdentifier(schemaName);
-        String objectIdentifier = parseIdentifier(objectName);
+        if (schemaName == null) {
+            schemaName = DEFAULT_SCHEMA_NAME;
+        }
 
-        verifySchemaIdentifier(schemaIdentifier);
-        verifyObjectIdentifier(objectIdentifier);
+        verifyObjectIdentifier(objectName);
+        verifySchemaIdentifier(schemaName);
+
+        String schemaIdentifier = parseSimpleName(schemaName);
+        String objectIdentifier = parseSimpleName(objectName);
 
         return new QualifiedName(schemaIdentifier, objectIdentifier);
     }
 
+    /**
+     * Return QualifiedName from a given schema and table normalized names.
+     *
+     * @param schemaName Normalized schema name or {@code null} for default schema.
+     * @param objectName Normalized object name.
+     * @return Qualified name.
+     */
+    public static QualifiedName fromNormalized(@Nullable String schemaName, String objectName) {
+        verifySchemaIdentifier(schemaName);
+        verifyObjectIdentifier(objectName);
+
+        return new QualifiedName(schemaName == null ? DEFAULT_SCHEMA_NAME : schemaName, objectName);
+    }
+
+

Review Comment:
   Done, thanks.



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with another TODO
   ```
   // TODO: https://issues.apache.org/jira/browse/IGNITE-24021
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks like a workaround, considering that we are passing the name of the schema, but reading object name (not schema name)



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with another TODO
   ```
   // TODO: https://issues.apache.org/jira/browse/IGNITE-24021
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks weird and a bit confusing, considering that we are passing the name of the schema



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/test/java/org/apache/ignite/table/QualifiedNameTest.java:
##########
@@ -273,9 +270,4 @@ public void malformedCanonicalNames(String source) {
                 containsString("Malformed identifier [identifier=" + source)
         )));
     }
-
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 Move to IgniteNameUtils
-    private static String canonicalName(String schemaName, String objectName) {
-        return schemaName == null ? quoteIfNeeded(objectName) : quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);

Review Comment:
   There is no reason to allow `null` for `schemaName`. We using `quoteIfNeeded` instead.
   
   Alternatively we can change method signature to 
   ```
   canonicalName(String ... names)
   ```
   and replace quoteIfNeeded(name) with canonicalName(name) everywhere.... thus making `quoteIfNeeded` private :thinking: 



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   This comment conflicts with
   ```
   //  Replace this with using correct implementation of `IgniteNameUtils.parseSimpleName`, when it will be fixed.
   String schemaNameInCanonicalForm = QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   Also 
   ```
   QualifiedName.fromSimple(defaultSchemaName).objectName();
   ```
   looks confusing, considering that we are passing the name of the schema



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/table/QualifiedName.java:
##########
@@ -107,15 +108,34 @@ public static QualifiedName fromSimple(String simpleName) {
      * @return Qualified name.
      */
     public static QualifiedName of(@Nullable String schemaName, String objectName) {
-        String schemaIdentifier = schemaName == null ? DEFAULT_SCHEMA_NAME : parseIdentifier(schemaName);
-        String objectIdentifier = parseIdentifier(objectName);
+        if (schemaName == null) {
+            schemaName = DEFAULT_SCHEMA_NAME;
+        }
 
-        verifySchemaIdentifier(schemaIdentifier);
-        verifyObjectIdentifier(objectIdentifier);
+        verifyObjectIdentifier(objectName);
+        verifySchemaIdentifier(schemaName);
+
+        String schemaIdentifier = parseSimpleName(schemaName);
+        String objectIdentifier = parseSimpleName(objectName);
 
         return new QualifiedName(schemaIdentifier, objectIdentifier);
     }
 
+    /**
+     * Return QualifiedName from a given schema and table normalized names.
+     *
+     * @param schemaName Normalized schema name or {@code null} for default schema.
+     * @param objectName Normalized object name.
+     * @return Qualified name.
+     */
+    public static QualifiedName fromNormalized(@Nullable String schemaName, String objectName) {
+        verifySchemaIdentifier(schemaName);
+        verifyObjectIdentifier(objectName);
+
+        return new QualifiedName(schemaName == null ? DEFAULT_SCHEMA_NAME : schemaName, objectName);
+    }
+
+

Review Comment:
   Fixed, thanks.



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   renamed



##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();

Review Comment:
   Done, thanks



##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {

Review Comment:
   Fixed, thanks



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -38,8 +37,6 @@ private IgniteNameUtils() {
      * @param name String to parse object name.
      * @return Unquoted name or name is cast to upper case. "tbl0" -&gt; "TBL0", "\"Tbl0\"" -&gt; "Tbl0".
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: Use QualifiedName instead.
-    @Deprecated(forRemoval = true)
     public static String parseSimpleName(String name) {

Review Comment:
   Let's rename method to  `parseIdentifier`



-- 
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


Re: [PR] IGNITE-24021 Sql. Merged duplicate code in QualifiedName and IgniteNameUtils [ignite-3]

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


##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {
+            return quote(identifier);
         }
 
+        for (int pos = 0; pos < identifier.length(); pos++) {
+            int codePoint = identifier.codePointAt(pos);
+
+            if (!identifierExtend(codePoint) && !Character.isUpperCase(codePoint)) {

Review Comment:
   ```suggestion
               if (!(identifierStart(codePoint) || identifierExtend(codePoint)) || !Character.isUpperCase(codePoint)) {
   ```



##########
modules/api/src/main/java/org/apache/ignite/lang/util/IgniteNameUtils.java:
##########
@@ -57,43 +52,78 @@ public static String parseSimpleName(String name) {
     }
 
     /**
-     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
+     * Parses fully qualified name.
      *
-     * @param schemaName Name of the schema.
-     * @param objectName Name of the object.
-     * @return Returns fully qualified name in canonical form.
+     * @param name Fully qualified name of the object in canonical form.
+     * @return List of identifiers, where each identifier within the full name chain will be either unquoted or converted to uppercase.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: replace `quote` call with `quoteIfNeeded`
-    @Deprecated(forRemoval = true)
-    public static String canonicalName(String schemaName, String objectName) {
-        return quote(schemaName) + '.' + quote(objectName);
+    public static List<String> parseName(String name) {
+        ensureNotNullAndNotEmpty(name, "name");
+
+        List<String> identifiers = new ArrayList<>();
+        Tokenizer tokenizer = new Tokenizer(name);
+
+        do {
+            identifiers.add(tokenizer.nextToken());
+        } while (tokenizer.hasNext());
+
+        return identifiers;
     }
 
     /**
-     * Tests if given string is fully qualified name in canonical form or simple name.
+     * Creates a fully qualified name in canonical form, that is, enclosing each part of the identifier chain in double quotes.
      *
-     * @param s String to test.
-     * @return {@code True} if given string is fully qualified name in canonical form or simple name.
+     * @param schemaName Normalized name of the schema.
+     * @param objectName Normalized name of the object.
+     * @return Returns fully qualified name in canonical form.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021: drop this method.
-    @Deprecated(forRemoval = true)
-    public static boolean canonicalOrSimpleName(String s) {
-        return NAME_PATTER.matcher(s).matches();
+    public static String canonicalName(String schemaName, String objectName) {
+        return quoteIfNeeded(schemaName) + '.' + quoteIfNeeded(objectName);
     }
 
     /**
-     * Wraps the given name with double quotes, e.g. "myColumn" -&gt; "\"myColumn\""
+     * Wraps the given name with double quotes if it not uppercased non-quoted name, e.g. "myColumn" -&gt; "\"myColumn\"", "MYCOLUMN" -&gt;
+     * "MYCOLUMN"
      *
-     * @param name Object name.
+     * @param identifier Object identifier.
      * @return Quoted object name.
      */
-    // TODO https://issues.apache.org/jira/browse/IGNITE-24021 make it private, `quoteIfNeeded` should be used instead.
-    @Deprecated
-    public static String quote(String name) {
-        if (name == null || name.isEmpty()) {
-            return name;
+    public static String quoteIfNeeded(String identifier) {
+        ensureNotNullAndNotEmpty(identifier, "identifier");
+
+        if (!identifierStart(identifier.codePointAt(0)) && !Character.isUpperCase(identifier.codePointAt(0))) {

Review Comment:
   ```suggestion
           if (!identifierStart(identifier.codePointAt(0)) || !Character.isUpperCase(identifier.codePointAt(0))) {
   ```



-- 
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