Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Parameters are in proper order for queries having WITH clause #17083

Merged
merged 1 commit into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ protected RowExpression toRowExpression(Expression expression, Session session)
new SqlParser(),
typeProvider,
expression,
ImmutableList.of(),
ImmutableMap.of(),
WarningCollector.NOOP);
return SqlToRowExpressionTranslator.translate(expression, expressionTypes, ImmutableMap.of(), functionAndTypeManager, session);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.facebook.presto.sql.tree.SymbolReference;
import com.google.common.base.VerifyException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

import javax.annotation.Nullable;
import javax.inject.Inject;
Expand Down Expand Up @@ -96,7 +97,7 @@
import static java.lang.Double.isNaN;
import static java.lang.Double.min;
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull;

public class FilterStatsCalculator
Expand Down Expand Up @@ -181,7 +182,7 @@ private Map<NodeRef<Expression>, Type> getExpressionTypes(Session session, Expre
metadata.getFunctionAndTypeManager(),
session,
types,
emptyList(),
emptyMap(),
node -> new IllegalStateException("Unexpected node: %s" + node),
WarningCollector.NOOP,
false);
Expand Down Expand Up @@ -463,7 +464,7 @@ private Type getType(Expression expression)
metadata.getFunctionAndTypeManager(),
session,
types,
ImmutableList.of(),
ImmutableMap.of(),
// At this stage, there should be no subqueries in the plan.
node -> new VerifyException("Unexpected subquery"),
WarningCollector.NOOP,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import com.facebook.presto.sql.tree.NullLiteral;
import com.facebook.presto.sql.tree.SymbolReference;
import com.facebook.presto.type.TypeUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

import javax.inject.Inject;

Expand All @@ -72,7 +72,7 @@
import static java.lang.Double.isNaN;
import static java.lang.Math.abs;
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull;

public class ScalarStatsCalculator
Expand Down Expand Up @@ -356,7 +356,7 @@ protected VariableStatsEstimate visitNullLiteral(NullLiteral node, Void context)
protected VariableStatsEstimate visitLiteral(Literal node, Void context)
{
Object value = evaluate(metadata, session.toConnectorSession(), node);
Type type = ExpressionAnalyzer.createConstantAnalyzer(metadata, session, ImmutableList.of(), WarningCollector.NOOP).analyze(node, Scope.create());
Type type = ExpressionAnalyzer.createConstantAnalyzer(metadata, session, ImmutableMap.of(), WarningCollector.NOOP).analyze(node, Scope.create());
OptionalDouble doubleValue = toStatsRepresentation(metadata, session, type, value);
VariableStatsEstimate.Builder estimate = VariableStatsEstimate.builder()
.setNullsFraction(0)
Expand Down Expand Up @@ -398,7 +398,7 @@ private Map<NodeRef<Expression>, Type> getExpressionTypes(Session session, Expre
metadata.getFunctionAndTypeManager(),
session,
types,
emptyList(),
emptyMap(),
node -> new IllegalStateException("Unexpected node: %s" + node),
WarningCollector.NOOP,
false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.facebook.presto.spi.relation.VariableReferenceExpression;
import com.facebook.presto.sql.planner.TypeProvider;
import com.facebook.presto.sql.planner.iterative.Lookup;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -87,7 +87,7 @@ private List<Object> getVariableValues(ValuesNode valuesNode, int symbolId, Sess
.map(row -> row.get(symbolId))
.map(rowExpression -> {
if (isExpression(rowExpression)) {
return evaluateConstantExpression(castToExpression(rowExpression), type, metadata, session, ImmutableList.of());
return evaluateConstantExpression(castToExpression(rowExpression), type, metadata, session, ImmutableMap.of());
}
return evaluateConstantRowExpression(rowExpression, metadata, session.toConnectorSession());
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
import static com.facebook.presto.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT;
import static com.facebook.presto.sql.NodeUtils.mapFromProperties;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.COLUMN_ALREADY_EXISTS;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
Expand Down Expand Up @@ -113,7 +114,7 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
sqlProperties,
session,
metadata,
parameters);
parameterExtractor(statement, parameters));

ColumnMetadata column = new ColumnMetadata(
element.getName().getValue(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,19 @@
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.tree.AlterFunction;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Parameter;
import com.facebook.presto.transaction.TransactionManager;
import com.google.common.util.concurrent.ListenableFuture;

import javax.inject.Inject;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import static com.facebook.presto.metadata.FunctionAndTypeManager.qualifyObjectName;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -64,7 +68,8 @@ public String explain(AlterFunction statement, List<Expression> parameters)
@Override
public ListenableFuture<?> execute(AlterFunction statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List<Expression> parameters, WarningCollector warningCollector)
{
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, warningCollector);
Map<NodeRef<Parameter>, Expression> parameterLookup = parameterExtractor(statement, parameters);
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, parameterLookup, warningCollector);
analyzer.analyze(statement);

QualifiedObjectName functionName = qualifyObjectName(statement.getFunctionName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import com.facebook.presto.sql.tree.CallArgument;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.ExpressionTreeRewriter;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Parameter;
import com.facebook.presto.transaction.TransactionManager;
import com.google.common.util.concurrent.ListenableFuture;

Expand All @@ -51,6 +53,7 @@
import static com.facebook.presto.spi.StandardErrorCode.INVALID_PROCEDURE_DEFINITION;
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
import static com.facebook.presto.spi.StandardErrorCode.PROCEDURE_CALL_FAILED;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.INVALID_PROCEDURE_ARGUMENTS;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_CATALOG;
import static com.facebook.presto.sql.planner.ExpressionInterpreter.evaluateConstantExpression;
Expand Down Expand Up @@ -128,16 +131,17 @@ else if (i < procedure.getArguments().size()) {

// get argument values
Object[] values = new Object[procedure.getArguments().size()];
Map<NodeRef<Parameter>, Expression> parameterLookup = parameterExtractor(call, parameters);
for (Entry<String, CallArgument> entry : names.entrySet()) {
CallArgument callArgument = entry.getValue();
int index = positions.get(entry.getKey());
Argument argument = procedure.getArguments().get(index);

Expression expression = ExpressionTreeRewriter.rewriteWith(new ParameterRewriter(parameters), callArgument.getValue());
Expression expression = ExpressionTreeRewriter.rewriteWith(new ParameterRewriter(parameterLookup), callArgument.getValue());
Type type = metadata.getType(argument.getType());
checkCondition(type != null, INVALID_PROCEDURE_DEFINITION, "Unknown procedure argument type: %s", argument.getType());

Object value = evaluateConstantExpression(expression, type, metadata, session, parameters);
Object value = evaluateConstantExpression(expression, type, metadata, session, parameterLookup);

values[index] = toTypeObjectValue(session, type, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.ExpressionRewriter;
import com.facebook.presto.sql.tree.ExpressionTreeRewriter;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Return;
import com.facebook.presto.sql.tree.RoutineBody;
import com.facebook.presto.transaction.TransactionManager;
Expand All @@ -53,6 +54,7 @@
import static com.facebook.presto.spi.StandardErrorCode.ALREADY_EXISTS;
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
import static com.facebook.presto.spi.function.FunctionVersion.notVersioned;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.SqlFormatter.formatSql;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.util.concurrent.Futures.immediateFuture;
Expand Down Expand Up @@ -86,7 +88,8 @@ public String explain(CreateFunction statement, List<Expression> parameters)
@Override
public ListenableFuture<?> execute(CreateFunction statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List<Expression> parameters, WarningCollector warningCollector)
{
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, warningCollector);
Map<NodeRef<com.facebook.presto.sql.tree.Parameter>, Expression> parameterLookup = parameterExtractor(statement, parameters);
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, parameterLookup, warningCollector);
Analysis analysis = analyzer.analyze(statement);
if (analysis.getFunctionHandles().values().stream()
.anyMatch(SqlFunctionHandle.class::isInstance)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.tree.CreateMaterializedView;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Parameter;
import com.facebook.presto.transaction.TransactionManager;
import com.google.common.util.concurrent.ListenableFuture;

Expand All @@ -46,6 +48,7 @@
import static com.facebook.presto.spi.StandardErrorCode.ALREADY_EXISTS;
import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
import static com.facebook.presto.sql.NodeUtils.mapFromProperties;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.SqlFormatterUtil.getFormattedSql;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MATERIALIZED_VIEW_ALREADY_EXISTS;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED;
Expand Down Expand Up @@ -86,7 +89,8 @@ public ListenableFuture<?> execute(CreateMaterializedView statement, Transaction
accessControl.checkCanCreateTable(session.getRequiredTransactionId(), session.getIdentity(), session.getAccessControlContext(), viewName);
accessControl.checkCanCreateView(session.getRequiredTransactionId(), session.getIdentity(), session.getAccessControlContext(), viewName);

Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, warningCollector);
Map<NodeRef<Parameter>, Expression> parameterLookup = parameterExtractor(statement, parameters);
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, parameterLookup, warningCollector);
Analysis analysis = analyzer.analyze(statement);

ConnectorId connectorId = metadata.getCatalogHandle(session, viewName.getCatalogName())
Expand All @@ -103,7 +107,7 @@ public ListenableFuture<?> execute(CreateMaterializedView statement, Transaction
sqlProperties,
session,
metadata,
parameters);
parameterLookup);

ConnectorTableMetadata viewMetadata = new ConnectorTableMetadata(
toSchemaTableName(viewName),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import static com.facebook.presto.metadata.MetadataUtil.createCatalogSchemaName;
import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
import static com.facebook.presto.sql.NodeUtils.mapFromProperties;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.SCHEMA_ALREADY_EXISTS;
import static com.google.common.util.concurrent.Futures.immediateFuture;

Expand Down Expand Up @@ -76,7 +77,7 @@ public ListenableFuture<?> execute(CreateSchema statement, TransactionManager tr
mapFromProperties(statement.getProperties()),
session,
metadata,
parameters);
parameterExtractor(statement, parameters));

metadata.createSchema(session, schema, properties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import com.facebook.presto.sql.tree.CreateTable;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.LikeClause;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Parameter;
import com.facebook.presto.sql.tree.TableElement;
import com.facebook.presto.transaction.TransactionManager;
import com.google.common.annotations.VisibleForTesting;
Expand All @@ -54,6 +56,7 @@
import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
import static com.facebook.presto.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT;
import static com.facebook.presto.sql.NodeUtils.mapFromProperties;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.DUPLICATE_COLUMN_NAME;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_CATALOG;
import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE;
Expand Down Expand Up @@ -89,6 +92,7 @@ public ListenableFuture<?> internalExecute(CreateTable statement, Metadata metad
{
checkArgument(!statement.getElements().isEmpty(), "no columns for table");

Map<NodeRef<Parameter>, Expression> parameterLookup = parameterExtractor(statement, parameters);
QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getName());
Optional<TableHandle> tableHandle = metadata.getTableHandle(session, tableName);
if (tableHandle.isPresent()) {
Expand Down Expand Up @@ -132,7 +136,7 @@ public ListenableFuture<?> internalExecute(CreateTable statement, Metadata metad
sqlProperties,
session,
metadata,
parameters);
parameterLookup);

columns.put(name, new ColumnMetadata(
name,
Expand Down Expand Up @@ -188,7 +192,7 @@ else if (element instanceof LikeClause) {
sqlProperties,
session,
metadata,
parameters);
parameterLookup);

Map<String, Object> finalProperties = combineProperties(sqlProperties.keySet(), properties, inheritedProperties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName;
import static com.facebook.presto.metadata.MetadataUtil.toSchemaTableName;
import static com.facebook.presto.metadata.ViewDefinition.ViewColumn;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.facebook.presto.sql.SqlFormatterUtil.getFormattedSql;
import static com.facebook.presto.sql.tree.CreateView.Security.INVOKER;
import static com.google.common.collect.ImmutableList.toImmutableList;
Expand Down Expand Up @@ -111,7 +112,7 @@ public ListenableFuture<?> execute(CreateView statement, TransactionManager tran

private Analysis analyzeStatement(Statement statement, Session session, Metadata metadata, AccessControl accessControl, List<Expression> parameters, WarningCollector warningCollector)
{
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, warningCollector);
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, parameterExtractor(statement, parameters), warningCollector);
return analyzer.analyze(statement);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.facebook.presto.sql.parser.SqlParser;
import com.facebook.presto.sql.tree.DropFunction;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.NodeRef;
import com.facebook.presto.sql.tree.Parameter;
import com.facebook.presto.transaction.TransactionManager;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Sets;
Expand All @@ -33,12 +35,14 @@
import javax.inject.Inject;

import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import static com.facebook.presto.metadata.FunctionAndTypeManager.qualifyObjectName;
import static com.facebook.presto.metadata.SessionFunctionHandle.SESSION_NAMESPACE;
import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND;
import static com.facebook.presto.sql.ParameterUtils.parameterExtractor;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static java.lang.String.format;
Expand Down Expand Up @@ -72,7 +76,8 @@ public String explain(DropFunction statement, List<Expression> parameters)
@Override
public ListenableFuture<?> execute(DropFunction statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List<Expression> parameters, WarningCollector warningCollector)
{
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, warningCollector);
Map<NodeRef<Parameter>, Expression> parameterLookup = parameterExtractor(statement, parameters);
Analyzer analyzer = new Analyzer(session, metadata, sqlParser, accessControl, Optional.empty(), parameters, parameterLookup, warningCollector);
analyzer.analyze(statement);
Optional<List<TypeSignature>> parameterTypes = statement.getParameterTypes().map(types -> types.stream().map(TypeSignature::parseTypeSignature).collect(toImmutableList()));

Expand Down
Loading