diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java index 41126d6df466d..091442f8aee6a 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java +++ b/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(); diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index eac9bdedb6c76..b3f76a6c63413 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/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())); } } diff --git a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java b/presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java index def2e5896d124..ad95d287070ea 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/rewrite/ShowQueriesRewrite.java +++ b/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 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 buildProperties( Object objectName, Optional columnName, diff --git a/presto-main/src/test/java/com/facebook/presto/server/TestHttpRequestSessionContext.java b/presto-main/src/test/java/com/facebook/presto/server/TestHttpRequestSessionContext.java index 24b34a17a6298..f4dbbb6d12dc3 100644 --- a/presto-main/src/test/java/com/facebook/presto/server/TestHttpRequestSessionContext.java +++ b/presto-main/src/test/java/com/facebook/presto/server/TestHttpRequestSessionContext.java @@ -161,7 +161,7 @@ public void testPreparedStatementsSpecialCharacters() .put(PRESTO_LANGUAGE, "zh-TW") .put(PRESTO_TIME_ZONE, "Asia/Taipei") .put(PRESTO_CLIENT_INFO, "null") - .put(PRESTO_PREPARED_STATEMENT, "query1=select * from tbl:ns") + .put(PRESTO_PREPARED_STATEMENT, "query1=select * from \"tbl:ns\"") .build(), "testRemote"); SqlParserOptions options = new SqlParserOptions(); diff --git a/presto-main/src/test/java/com/facebook/presto/sql/TestSqlFormatter.java b/presto-main/src/test/java/com/facebook/presto/sql/TestSqlFormatter.java new file mode 100644 index 0000000000000..9e6154021203c --- /dev/null +++ b/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); + } +} diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java index d677404ba132a..406cafc7e19a1 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java @@ -123,7 +123,6 @@ import java.util.Iterator; import java.util.List; import java.util.Optional; -import java.util.regex.Pattern; import java.util.stream.Collectors; import static com.facebook.presto.sql.ExpressionFormatter.formatExpression; @@ -138,7 +137,6 @@ public final class SqlFormatter { private static final String INDENT = " "; - private static final Pattern NAME_PATTERN = Pattern.compile("[a-z_][a-z0-9_]*"); private SqlFormatter() {} @@ -1079,12 +1077,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) diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java b/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java index e7d51af5a1604..e160e712dc690 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java +++ b/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 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) diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/DereferenceExpression.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/DereferenceExpression.java index 351113137d229..ae9ec314dfb5a 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/DereferenceExpression.java +++ b/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 parts = tryParseParts(expression.base, expression.field.getValue().toLowerCase(Locale.ENGLISH)); + List 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 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 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 newList = new ArrayList<>(baseQualifiedName.getParts()); - newList.add(fieldName); - return newList; - } - } - return null; - } - @Override public boolean equals(Object o) { diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/Identifier.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Identifier.java index 022daa1870994..f6a52f4a4431b 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/Identifier.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/Identifier.java @@ -25,7 +25,7 @@ public class Identifier extends Expression { - private static final Pattern NAME_PATTERN = Pattern.compile("[a-zA-Z_]([a-zA-Z0-9_:@])*"); + private static final Pattern NAME_PATTERN = Pattern.compile("[a-zA-Z_]([a-zA-Z0-9_])*"); private final String value; private final boolean delimited; diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/QualifiedName.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/QualifiedName.java index 1a9f17a048b13..8bd65d692691f 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/QualifiedName.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/QualifiedName.java @@ -20,43 +20,43 @@ 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 parts; - private final List originalParts; + private final List 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 originalParts) + public static QualifiedName of(Iterable originalParts) { requireNonNull(originalParts, "originalParts is null"); checkArgument(!isEmpty(originalParts), "originalParts is empty"); - List 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 originalParts, List parts) + private QualifiedName(List originalParts) { this.originalParts = originalParts; - this.parts = parts; + this.parts = originalParts.stream().map(identifier -> identifier.getValue().toLowerCase(ENGLISH)).collect(toImmutableList()); } public List getParts() @@ -64,7 +64,7 @@ public List getParts() return parts; } - public List getOriginalParts() + public List getOriginalParts() { return originalParts; } @@ -85,8 +85,8 @@ public Optional getPrefix() return Optional.empty(); } - List subList = parts.subList(0, parts.size() - 1); - return Optional.of(new QualifiedName(subList, subList)); + List subList = originalParts.subList(0, originalParts.size() - 1); + return Optional.of(new QualifiedName(subList)); } public boolean hasSuffix(QualifiedName suffix) diff --git a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java index ef659818d3ed5..fe2b64ce5d7db 100644 --- a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java +++ b/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; @@ -617,7 +618,7 @@ public void testPrecedenceAndAssociativity() public void testAllowIdentifierColon() { SqlParser sqlParser = new SqlParser(new SqlParserOptions().allowIdentifierSymbol(COLON)); - sqlParser.createStatement("select * from foo:bar"); + sqlParser.createStatement("select * from \"foo:bar\""); } @SuppressWarnings("deprecation") @@ -625,7 +626,7 @@ public void testAllowIdentifierColon() public void testAllowIdentifierAtSign() { SqlParser sqlParser = new SqlParser(new SqlParserOptions().allowIdentifierSymbol(AT_SIGN)); - sqlParser.createStatement("select * from foo@bar"); + sqlParser.createStatement("select * from \"foo@bar\""); } @Test @@ -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 parts = Arrays.stream(tableName.split("\\.")) + .map(Identifier::new) + .collect(Collectors.toList()); + return QualifiedName.of(parts); + } + private ShowStats createShowStats(QualifiedName name, List selects, Optional where) { return new ShowStats( diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/IgnoredFunctionsMismatchResolver.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/IgnoredFunctionsMismatchResolver.java index a3424efac2e80..de20105100dec 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/resolver/IgnoredFunctionsMismatchResolver.java +++ b/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 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) diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriteConfig.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriteConfig.java index 9c524f959943a..8db59b7c3a1ff 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriteConfig.java +++ b/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; } diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java index 00c91700975d9..e9a0c68011f35 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/rewrite/QueryRewriter.java +++ b/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 originalName, QualifiedName prefix) { - List parts = new ArrayList<>(); + List 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); }