From 001dbd525f935c6d2f178c6eb84638efc9d53b79 Mon Sep 17 00:00:00 2001 From: Rohit Jain Date: Mon, 19 Dec 2022 17:56:25 -0800 Subject: [PATCH] Remove ParsingOptions from AnalyzerOptions Usage of the ParsingOptions creates a dependency on the presto-parser for analyzer interfaces. Presto analyzer interfaces would move to the presto-spi package and should be independent of the presto-parser. Hence removing ParsionOptions from the AnalyzerOptions. --- .../presto/sql/analyzer/AnalyzerOptions.java | 38 +++++++++++++------ .../sql/analyzer/BuiltInQueryPreparer.java | 5 ++- .../sql/analyzer/utils/AnalyzerUtil.java | 35 +++++++++++++++++ .../facebook/presto/util/AnalyzerUtil.java | 4 +- 4 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/AnalyzerUtil.java diff --git a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/AnalyzerOptions.java b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/AnalyzerOptions.java index 850ac4dd44b45..2fe580f508750 100644 --- a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/AnalyzerOptions.java +++ b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/AnalyzerOptions.java @@ -14,32 +14,36 @@ package com.facebook.presto.sql.analyzer; import com.facebook.presto.common.WarningHandlingLevel; -import com.facebook.presto.sql.parser.ParsingOptions; +import com.facebook.presto.spi.WarningCollector; import static com.facebook.presto.common.WarningHandlingLevel.NORMAL; +import static java.util.Objects.requireNonNull; /** * Various options required at different stage of query analysis. */ public class AnalyzerOptions { - private final ParsingOptions parsingOptions; + private final boolean isParseDecimalLiteralsAsDouble; private final boolean isLogFormattedQueryEnabled; private final WarningHandlingLevel warningHandlingLevel; + private final WarningCollector warningCollector; private AnalyzerOptions( - ParsingOptions parsingOptions, + boolean isParseDecimalLiteralsAsDouble, boolean isLogFormattedQueryEnabled, + WarningCollector warningCollector, WarningHandlingLevel warningHandlingLevel) { - this.parsingOptions = parsingOptions; + this.isParseDecimalLiteralsAsDouble = isParseDecimalLiteralsAsDouble; this.isLogFormattedQueryEnabled = isLogFormattedQueryEnabled; - this.warningHandlingLevel = warningHandlingLevel; + this.warningCollector = requireNonNull(warningCollector, "warningCollector is null"); + this.warningHandlingLevel = requireNonNull(warningHandlingLevel, "warningHandlingLevel is null"); } - public ParsingOptions getParsingOptions() + public boolean isParseDecimalLiteralsAsDouble() { - return parsingOptions; + return isParseDecimalLiteralsAsDouble; } public boolean isLogFormattedQueryEnabled() @@ -47,6 +51,11 @@ public boolean isLogFormattedQueryEnabled() return isLogFormattedQueryEnabled; } + public WarningCollector getWarningCollector() + { + return warningCollector; + } + public WarningHandlingLevel getWarningHandlingLevel() { return warningHandlingLevel; @@ -59,15 +68,16 @@ public static Builder builder() public static class Builder { - private ParsingOptions parsingOptions = ParsingOptions.builder().build(); + private boolean isParseDecimalLiteralsAsDouble; private boolean isLogFormattedQueryEnabled; + private WarningCollector warningCollector; private WarningHandlingLevel warningHandlingLevel = NORMAL; private Builder() {} - public Builder setParsingOptions(ParsingOptions parsingOptions) + public Builder setParseDecimalLiteralsAsDouble(boolean parseDecimalLiteralsAsDouble) { - this.parsingOptions = parsingOptions; + isParseDecimalLiteralsAsDouble = parseDecimalLiteralsAsDouble; return this; } @@ -77,6 +87,12 @@ public Builder setLogFormattedQueryEnabled(boolean logFormattedQueryEnabled) return this; } + public Builder setWarningCollector(WarningCollector warningCollector) + { + this.warningCollector = warningCollector; + return this; + } + public Builder setWarningHandlingLevel(WarningHandlingLevel warningHandlingLevel) { this.warningHandlingLevel = warningHandlingLevel; @@ -85,7 +101,7 @@ public Builder setWarningHandlingLevel(WarningHandlingLevel warningHandlingLevel public AnalyzerOptions build() { - return new AnalyzerOptions(parsingOptions, isLogFormattedQueryEnabled, warningHandlingLevel); + return new AnalyzerOptions(isParseDecimalLiteralsAsDouble, isLogFormattedQueryEnabled, warningCollector, warningHandlingLevel); } } } diff --git a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/BuiltInQueryPreparer.java b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/BuiltInQueryPreparer.java index 5aa0b465e17c1..6f12ed42034f9 100644 --- a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/BuiltInQueryPreparer.java +++ b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/BuiltInQueryPreparer.java @@ -40,6 +40,7 @@ import static com.facebook.presto.sql.SqlFormatter.formatSql; import static com.facebook.presto.sql.analyzer.ConstantExpressionVerifier.verifyExpressionIsConstant; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_PARAMETER_USAGE; +import static com.facebook.presto.sql.analyzer.utils.AnalyzerUtil.createParsingOptions; import static com.facebook.presto.sql.analyzer.utils.ParameterExtractor.getParameterCount; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -63,7 +64,7 @@ public BuiltInQueryPreparer(SqlParser sqlParser) public BuiltInPreparedQuery prepareQuery(AnalyzerOptions analyzerOptions, String query, Map preparedStatements, WarningCollector warningCollector) throws ParsingException, PrestoException, SemanticException { - Statement wrappedStatement = sqlParser.createStatement(query, analyzerOptions.getParsingOptions()); + Statement wrappedStatement = sqlParser.createStatement(query, createParsingOptions(analyzerOptions)); if (warningCollector.hasWarnings() && analyzerOptions.getWarningHandlingLevel() == AS_ERROR) { throw new PrestoException(WARNING_AS_ERROR, format("Warning handling level set to AS_ERROR. Warnings: %n %s", warningCollector.getWarnings().stream() @@ -82,7 +83,7 @@ public BuiltInPreparedQuery prepareQuery(AnalyzerOptions analyzerOptions, Statem String preparedStatementName = ((Execute) statement).getName().getValue(); prepareSql = Optional.ofNullable(preparedStatements.get(preparedStatementName)); String query = prepareSql.orElseThrow(() -> new PrestoException(NOT_FOUND, "Prepared statement not found: " + preparedStatementName)); - statement = sqlParser.createStatement(query, analyzerOptions.getParsingOptions()); + statement = sqlParser.createStatement(query, createParsingOptions(analyzerOptions)); } if (statement instanceof Explain && ((Explain) statement).isAnalyze()) { diff --git a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/AnalyzerUtil.java b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/AnalyzerUtil.java new file mode 100644 index 0000000000000..c16588fcbe4a2 --- /dev/null +++ b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/AnalyzerUtil.java @@ -0,0 +1,35 @@ +/* + * 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.analyzer.utils; + +import com.facebook.presto.spi.PrestoWarning; +import com.facebook.presto.sql.analyzer.AnalyzerOptions; +import com.facebook.presto.sql.parser.ParsingOptions; + +import static com.facebook.presto.spi.StandardWarningCode.PARSER_WARNING; +import static com.facebook.presto.sql.parser.ParsingOptions.DecimalLiteralTreatment.AS_DECIMAL; +import static com.facebook.presto.sql.parser.ParsingOptions.DecimalLiteralTreatment.AS_DOUBLE; + +public class AnalyzerUtil +{ + private AnalyzerUtil() {} + + public static ParsingOptions createParsingOptions(AnalyzerOptions analyzerOptions) + { + return ParsingOptions.builder() + .setDecimalLiteralTreatment(analyzerOptions.isParseDecimalLiteralsAsDouble() ? AS_DOUBLE : AS_DECIMAL) + .setWarningConsumer(warning -> analyzerOptions.getWarningCollector().add(new PrestoWarning(PARSER_WARNING, warning.getMessage()))) + .build(); + } +} diff --git a/presto-main/src/main/java/com/facebook/presto/util/AnalyzerUtil.java b/presto-main/src/main/java/com/facebook/presto/util/AnalyzerUtil.java index a5f45138f7e39..255bc56fb0a61 100644 --- a/presto-main/src/main/java/com/facebook/presto/util/AnalyzerUtil.java +++ b/presto-main/src/main/java/com/facebook/presto/util/AnalyzerUtil.java @@ -55,11 +55,11 @@ public static AnalyzerOptions createAnalyzerOptions(Session session) public static AnalyzerOptions createAnalyzerOptions(Session session, WarningCollector warningCollector) { - ParsingOptions parsingOptions = createParsingOptions(session, warningCollector); return AnalyzerOptions.builder() - .setParsingOptions(parsingOptions) + .setParseDecimalLiteralsAsDouble(isParseDecimalLiteralsAsDouble(session)) .setLogFormattedQueryEnabled(isLogFormattedQueryEnabled(session)) .setWarningHandlingLevel(getWarningHandlingLevel(session)) + .setWarningCollector(warningCollector) .build(); } }