Preserve quotedness for Identifier when formatting Query

Cherry-pick of trinodb/trino#80

Presto doesn't maintain the quotedness of an identifier when
using SqlQueryFormatter so it results in throwing parsing error
when we run prepare query of the syntax
[#10739].
This PR solves that above issue.

Co-authored-by: praveenkrishna <praveenkrishna@tutanota.com>

presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java

@@ -152,11 +152,6 @@ public static QualifiedObjectName createQualifiedObjectName(Session session, Nod
         return new QualifiedObjectName(catalogName, schemaName, objectName);
     }
 
-    public static QualifiedName createQualifiedName(QualifiedObjectName name)
-    {
-        return QualifiedName.of(name.getCatalogName(), name.getSchemaName(), name.getObjectName());
-    }
-
     public static PrestoPrincipal createPrincipal(Session session, GrantorSpecification specification)
     {
         GrantorSpecification.Type type = specification.getType();

presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java

@@ -2281,7 +2281,7 @@ else if (expression instanceof DereferenceExpression) {
 
                     if (!field.isPresent()) {
                         if (name != null) {
-                            field = Optional.of(new Identifier(getLast(name.getOriginalParts())));
+                            field = Optional.of(getLast(name.getOriginalParts()));
                         }
                     }
 

presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java

@@ -105,7 +105,6 @@
 import static com.facebook.presto.metadata.MetadataListing.listCatalogs;
 import static com.facebook.presto.metadata.MetadataListing.listSchemas;
 import static com.facebook.presto.metadata.MetadataUtil.createCatalogSchemaName;
-import static com.facebook.presto.metadata.MetadataUtil.createQualifiedName;
 import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
 import static com.facebook.presto.metadata.SessionFunctionHandle.SESSION_NAMESPACE;
 import static com.facebook.presto.spi.StandardErrorCode.FUNCTION_NOT_FOUND;
@@ -459,7 +458,7 @@ protected Node visitShowCreate(ShowCreate node, Void context)
                 }
 
                 Query query = parseView(viewDefinition.get().getOriginalSql(), objectName, node);
-                String sql = formatSql(new CreateView(createQualifiedName(objectName), query, false, Optional.empty()), Optional.of(parameters)).trim();
+                String sql = formatSql(new CreateView(getQualifiedName(node, objectName), query, false, Optional.empty()), Optional.of(parameters)).trim();
                 return singleValueQuery("Create View", sql);
             }
 
@@ -485,7 +484,7 @@ protected Node visitShowCreate(ShowCreate node, Void context)
 
                 CreateMaterializedView createMaterializedView = new CreateMaterializedView(
                         Optional.empty(),
-                        createQualifiedName(objectName),
+                        getQualifiedName(node, objectName),
                         query,
                         false,
                         propertyNodes,
@@ -593,6 +592,16 @@ protected Node visitShowCreateFunction(ShowCreateFunction node, Void context)
                     ordering(ascending("argument_types")));
         }
 
+        private QualifiedName getQualifiedName(ShowCreate node, QualifiedObjectName objectName)
+        {
+            List<Identifier> parts = node.getName().getOriginalParts();
+            Identifier tableName = parts.get(0);
+            Identifier schemaName = (parts.size() > 1) ? parts.get(1) : new Identifier(objectName.getSchemaName());
+            Identifier catalogName = (parts.size() > 2) ? parts.get(2) : new Identifier(objectName.getCatalogName());
+
+            return QualifiedName.of(ImmutableList.of(catalogName, schemaName, tableName));
+        }
+
         private List<Property> buildProperties(
                 Object objectName,
                 Optional<String> columnName,

presto-main/src/test/java/com/facebook/presto/sql/TestSqlFormatter.java

@@ -0,0 +1,43 @@
+/*
+ * Licensed 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 com.facebook.presto.sql;
+
+import com.facebook.presto.sql.parser.ParsingOptions;
+import com.facebook.presto.sql.parser.SqlParser;
+import com.facebook.presto.sql.tree.Statement;
+import org.testng.annotations.Test;
+
+import java.util.Optional;
+
+import static com.facebook.presto.sql.SqlFormatterUtil.getFormattedSql;
+import static org.testng.Assert.assertEquals;
+
+public class TestSqlFormatter
+{
+    @Test
+    public void testSimpleExpression()
+    {
+        assetQuery("SELECT id\nFROM\n  public.orders\n");
+        assetQuery("SELECT id\nFROM\n  \"public\".\"order\"\n");
+        assetQuery("SELECT id\nFROM\n  \"public\".\"order\"\"2\"\n");
+    }
+
+    private void assetQuery(String query)
+    {
+        SqlParser parser = new SqlParser();
+        Statement statement = parser.createStatement(query, new ParsingOptions());
+        String formattedQuery = getFormattedSql(statement, parser, Optional.empty());
+        assertEquals(formattedQuery, query);
+    }
+}

presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java

@@ -1079,12 +1079,10 @@ private String formatRoutineCharacteristicName(Enum characteristic)
             return characteristic.name().replace("_", " ");
         }
 
-        private static String formatName(String name)
+        private static String formatName(Identifier name)
         {
-            if (NAME_PATTERN.matcher(name).matches()) {
-                return name;
-            }
-            return "\"" + name.replace("\"", "\"\"") + "\"";
+            String delimiter = name.isDelimited() ? "\"" : "";
+            return delimiter + name.getValue().replace("\"", "\"\"") + delimiter;
         }
 
         private static String formatName(QualifiedName name)

presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java

@@ -2116,11 +2116,7 @@ private static LikeClause.PropertiesOption getPropertiesOption(Token token)
 
     private QualifiedName getQualifiedName(SqlBaseParser.QualifiedNameContext context)
     {
-        List<String> parts = visit(context.identifier(), Identifier.class).stream()
-                .map(Identifier::getValue) // TODO: preserve quotedness
-                .collect(Collectors.toList());
-
-        return QualifiedName.of(parts);
+        return QualifiedName.of(visit(context.identifier(), Identifier.class));
     }
 
     private static boolean isDistinct(SqlBaseParser.SetQuantifierContext setQuantifier)

presto-parser/src/main/java/com/facebook/presto/sql/tree/DereferenceExpression.java

@@ -15,9 +15,7 @@
 
 import com.google.common.collect.ImmutableList;
 
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
 
@@ -76,7 +74,20 @@ public Identifier getField()
      */
     public static QualifiedName getQualifiedName(DereferenceExpression expression)
     {
-        List<String> parts = tryParseParts(expression.base, expression.field.getValue().toLowerCase(Locale.ENGLISH));
+        List<Identifier> parts = null;
+        if (expression.base instanceof Identifier) {
+            parts = ImmutableList.of((Identifier) expression.base, expression.field);
+        }
+        else if (expression.base instanceof DereferenceExpression) {
+            QualifiedName baseQualifiedName = getQualifiedName((DereferenceExpression) expression.base);
+            if (baseQualifiedName != null) {
+                ImmutableList.Builder<Identifier> builder = ImmutableList.builder();
+                builder.addAll(baseQualifiedName.getOriginalParts());
+                builder.add(expression.field);
+                parts = builder.build();
+            }
+        }
+
         return parts == null ? null : QualifiedName.of(parts);
     }
 
@@ -96,22 +107,6 @@ public static Expression from(QualifiedName name)
         return result;
     }
 
-    private static List<String> tryParseParts(Expression base, String fieldName)
-    {
-        if (base instanceof Identifier) {
-            return ImmutableList.of(((Identifier) base).getValue(), fieldName);
-        }
-        else if (base instanceof DereferenceExpression) {
-            QualifiedName baseQualifiedName = getQualifiedName((DereferenceExpression) base);
-            if (baseQualifiedName != null) {
-                List<String> newList = new ArrayList<>(baseQualifiedName.getParts());
-                newList.add(fieldName);
-                return newList;
-            }
-        }
-        return null;
-    }
-
     @Override
     public boolean equals(Object o)
     {

presto-parser/src/main/java/com/facebook/presto/sql/tree/QualifiedName.java

@@ -20,51 +20,51 @@
 
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.Iterables.isEmpty;
-import static com.google.common.collect.Iterables.transform;
 import static java.util.Locale.ENGLISH;
 import static java.util.Objects.requireNonNull;
 
 public class QualifiedName
 {
     private final List<String> parts;
-    private final List<String> originalParts;
+    private final List<Identifier> originalParts;
 
     public static QualifiedName of(String first, String... rest)
     {
         requireNonNull(first, "first is null");
-        return of(ImmutableList.copyOf(Lists.asList(first, rest)));
+        return of(ImmutableList.copyOf(Lists.asList(first, rest).stream().map(Identifier::new).collect(Collectors.toList())));
     }
 
     public static QualifiedName of(String name)
     {
         requireNonNull(name, "name is null");
-        return of(ImmutableList.of(name));
+        return of(ImmutableList.of(new Identifier(name)));
     }
 
-    public static QualifiedName of(Iterable<String> originalParts)
+    public static QualifiedName of(Iterable<Identifier> originalParts)
     {
         requireNonNull(originalParts, "originalParts is null");
         checkArgument(!isEmpty(originalParts), "originalParts is empty");
-        List<String> parts = ImmutableList.copyOf(transform(originalParts, part -> part.toLowerCase(ENGLISH)));
 
-        return new QualifiedName(ImmutableList.copyOf(originalParts), parts);
+        return new QualifiedName(ImmutableList.copyOf(originalParts));
     }
 
-    private QualifiedName(List<String> originalParts, List<String> parts)
+    private QualifiedName(List<Identifier> originalParts)
     {
         this.originalParts = originalParts;
-        this.parts = parts;
+        this.parts = originalParts.stream().map(identifier -> identifier.getValue().toLowerCase(ENGLISH)).collect(toImmutableList());
     }
 
     public List<String> getParts()
     {
         return parts;
     }
 
-    public List<String> getOriginalParts()
+    public List<Identifier> getOriginalParts()
     {
         return originalParts;
     }
@@ -85,8 +85,8 @@ public Optional<QualifiedName> getPrefix()
             return Optional.empty();
         }
 
-        List<String> subList = parts.subList(0, parts.size() - 1);
-        return Optional.of(new QualifiedName(subList, subList));
+        List<Identifier> subList = originalParts.subList(0, originalParts.size() - 1);
+        return Optional.of(new QualifiedName(subList));
     }
 
     public boolean hasSuffix(QualifiedName suffix)

presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java

@@ -157,6 +157,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 import static com.facebook.presto.sql.QueryUtil.identifier;
 import static com.facebook.presto.sql.QueryUtil.query;
@@ -2285,7 +2286,7 @@ public void testShowStats()
         final String[] tableNames = {"t", "s.t", "c.s.t"};
 
         for (String fullName : tableNames) {
-            QualifiedName qualifiedName = QualifiedName.of(Arrays.asList(fullName.split("\\.")));
+            QualifiedName qualifiedName = makeQualifiedName(fullName);
             assertStatement(format("SHOW STATS FOR %s", qualifiedName), new ShowStats(new Table(qualifiedName)));
         }
     }
@@ -2296,7 +2297,7 @@ public void testShowStatsForQuery()
         final String[] tableNames = {"t", "s.t", "c.s.t"};
 
         for (String fullName : tableNames) {
-            QualifiedName qualifiedName = QualifiedName.of(Arrays.asList(fullName.split("\\.")));
+            QualifiedName qualifiedName = makeQualifiedName(fullName);
             assertStatement(format("SHOW STATS FOR (SELECT * FROM %s)", qualifiedName),
                     createShowStats(qualifiedName, ImmutableList.of(new AllColumns()), Optional.empty()));
             assertStatement(format("SHOW STATS FOR (SELECT * FROM %s WHERE field > 0)", qualifiedName),
@@ -2320,6 +2321,14 @@ public void testShowStatsForQuery()
         }
     }
 
+    private QualifiedName makeQualifiedName(String tableName)
+    {
+        List<Identifier> parts = Arrays.stream(tableName.split("\\."))
+                .map(Identifier::new)
+                .collect(Collectors.toList());
+        return QualifiedName.of(parts);
+    }
+
     private ShowStats createShowStats(QualifiedName name, List<SelectItem> selects, Optional<Expression> where)
     {
         return new ShowStats(

presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/IgnoredFunctionsMismatchResolver.java

@@ -16,18 +16,20 @@
 import com.facebook.presto.common.CatalogSchemaName;
 import com.facebook.presto.sql.tree.AstVisitor;
 import com.facebook.presto.sql.tree.FunctionCall;
+import com.facebook.presto.sql.tree.Identifier;
 import com.facebook.presto.sql.tree.Node;
 import com.facebook.presto.sql.tree.QualifiedName;
 import com.facebook.presto.verifier.framework.DataMatchResult;
 import com.facebook.presto.verifier.framework.QueryBundle;
-import com.google.common.base.Splitter;
 
 import javax.inject.Inject;
 
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static com.facebook.presto.metadata.BuiltInTypeAndFunctionNamespaceManager.DEFAULT_NAMESPACE;
 import static com.facebook.presto.verifier.framework.DataMatchResult.MatchType.COLUMN_MISMATCH;
@@ -87,7 +89,9 @@ protected Void visitFunctionCall(FunctionCall node, Set<String> ignoredFunctions
 
     private static String normalizeFunctionName(String name)
     {
-        return normalizeFunctionName(QualifiedName.of(Splitter.on(".").splitToList(name)));
+        return normalizeFunctionName(QualifiedName.of(Arrays.stream(name.split("\\."))
+                .map(Identifier::new)
+                .collect(Collectors.toList())));
     }
 
     private static String normalizeFunctionName(QualifiedName name)

presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriteConfig.java

@@ -15,16 +15,18 @@
 
 import com.facebook.airlift.configuration.Config;
 import com.facebook.airlift.configuration.ConfigDescription;
+import com.facebook.presto.sql.tree.Identifier;
 import com.facebook.presto.sql.tree.QualifiedName;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableMap;
 
 import javax.validation.constraints.NotNull;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 public class QueryRewriteConfig
 {
@@ -43,7 +45,9 @@ public QueryRewriteConfig setTablePrefix(String tablePrefix)
     {
         this.tablePrefix = tablePrefix == null ?
                 null :
-                QualifiedName.of(Splitter.on(".").splitToList(tablePrefix));
+                QualifiedName.of(Arrays.stream(tablePrefix.split("\\."))
+                        .map(Identifier::new)
+                        .collect(Collectors.toList()));
         return this;
     }
 

presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java

@@ -228,14 +228,14 @@ public QueryObjectBundle rewriteQuery(@Language("SQL") String query, ClusterType
 
     private QualifiedName generateTemporaryName(Optional<QualifiedName> originalName, QualifiedName prefix)
     {
-        List<String> parts = new ArrayList<>();
+        List<Identifier> parts = new ArrayList<>();
         int originalSize = originalName.map(QualifiedName::getOriginalParts).map(List::size).orElse(0);
         int prefixSize = prefix.getOriginalParts().size();
         if (originalName.isPresent() && originalSize > prefixSize) {
             parts.addAll(originalName.get().getOriginalParts().subList(0, originalSize - prefixSize));
         }
         parts.addAll(prefix.getOriginalParts());
-        parts.set(parts.size() - 1, prefix.getSuffix() + "_" + randomUUID().toString().replace("-", ""));
+        parts.set(parts.size() - 1, new Identifier(prefix.getSuffix() + "_" + randomUUID().toString().replace("-", "")));
         return QualifiedName.of(parts);
     }