From 161b2721aaf211b4c96f2f5fcb33bbac1d989bba Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Fri, 4 Nov 2022 14:51:17 -0700 Subject: [PATCH 1/5] Rename parameterExtractor to bindParameters for clarity --- .../src/main/java/io/trino/execution/AddColumnTask.java | 4 ++-- .../src/main/java/io/trino/execution/CallTask.java | 4 ++-- .../java/io/trino/execution/CreateMaterializedViewTask.java | 4 ++-- .../src/main/java/io/trino/execution/CreateSchemaTask.java | 4 ++-- .../src/main/java/io/trino/execution/CreateTableTask.java | 4 ++-- .../src/main/java/io/trino/execution/CreateViewTask.java | 4 ++-- .../src/main/java/io/trino/execution/SetPropertiesTask.java | 6 +++--- .../src/main/java/io/trino/execution/SetSessionTask.java | 4 ++-- .../src/main/java/io/trino/execution/SetTimeZoneTask.java | 4 ++-- .../src/main/java/io/trino/execution/SqlQueryExecution.java | 4 ++-- .../src/main/java/io/trino/sql/ParameterUtils.java | 4 ++-- .../src/main/java/io/trino/sql/analyzer/QueryExplainer.java | 4 ++-- .../src/main/java/io/trino/testing/LocalQueryRunner.java | 4 ++-- 13 files changed, 27 insertions(+), 27 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java index c44acc566f5d..7a259895587b 100644 --- a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java @@ -44,7 +44,7 @@ import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.spi.StandardErrorCode.TYPE_NOT_FOUND; import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature; import static io.trino.type.UnknownType.UNKNOWN; @@ -122,7 +122,7 @@ public ListenableFuture execute( session, plannerContext, accessControl, - parameterExtractor(statement, parameters), + bindParameters(statement, parameters), true); ColumnMetadata column = ColumnMetadata.builder() diff --git a/core/trino-main/src/main/java/io/trino/execution/CallTask.java b/core/trino-main/src/main/java/io/trino/execution/CallTask.java index 1942126f2b5d..c63da0c3d066 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CallTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CallTask.java @@ -62,7 +62,7 @@ import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.PROCEDURE_CALL_FAILED; import static io.trino.spi.type.TypeUtils.writeNativeValue; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; import static java.util.Arrays.asList; @@ -156,7 +156,7 @@ else if (i < procedure.getArguments().size()) { // get argument values Object[] values = new Object[procedure.getArguments().size()]; - Map, Expression> parameterLookup = parameterExtractor(call, parameters); + Map, Expression> parameterLookup = bindParameters(call, parameters); for (Entry entry : names.entrySet()) { CallArgument callArgument = entry.getValue(); int index = positions.get(entry.getKey()); diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java index cd0c01b93165..0195d3451dab 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java @@ -41,7 +41,7 @@ import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.SqlFormatterUtil.getFormattedSql; import static java.util.Objects.requireNonNull; @@ -84,7 +84,7 @@ public ListenableFuture execute( { Session session = stateMachine.getSession(); QualifiedObjectName name = createQualifiedObjectName(session, statement, statement.getName()); - Map, Expression> parameterLookup = parameterExtractor(statement, parameters); + Map, Expression> parameterLookup = bindParameters(statement, parameters); String sql = getFormattedSql(statement.getQuery(), sqlParser); diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java index 7baaf0f5d5db..28d19b89f205 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java @@ -42,7 +42,7 @@ import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.SCHEMA_ALREADY_EXISTS; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static java.util.Objects.requireNonNull; @@ -109,7 +109,7 @@ static ListenableFuture internalExecute( session, plannerContext, accessControl, - parameterExtractor(statement, parameters), + bindParameters(statement, parameters), true); TrinoPrincipal principal = getCreatePrincipal(statement, session, plannerContext.getMetadata(), catalogName); diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java index dd2bb3cefb0a..93ae644eed3f 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java @@ -73,7 +73,7 @@ import static io.trino.spi.StandardErrorCode.TYPE_NOT_FOUND; import static io.trino.spi.StandardErrorCode.UNSUPPORTED_TABLE_TYPE; import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature; import static io.trino.sql.tree.LikeClause.PropertiesOption.EXCLUDING; @@ -124,7 +124,7 @@ ListenableFuture internalExecute(CreateTable statement, Session session, L { checkArgument(!statement.getElements().isEmpty(), "no columns for table"); - Map, Expression> parameterLookup = parameterExtractor(statement, parameters); + Map, Expression> parameterLookup = bindParameters(statement, parameters); QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getName()); Optional tableHandle; try { diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java index cd5b35b994d8..612316662dd8 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java @@ -37,7 +37,7 @@ import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.spi.StandardErrorCode.TABLE_ALREADY_EXISTS; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.SqlFormatterUtil.getFormattedSql; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.tree.CreateView.Security.INVOKER; @@ -92,7 +92,7 @@ else if (metadata.getTableHandle(session, name).isPresent()) { String sql = getFormattedSql(statement.getQuery(), sqlParser); - Analysis analysis = analyzerFactory.createAnalyzer(session, parameters, parameterExtractor(statement, parameters), stateMachine.getWarningCollector()) + Analysis analysis = analyzerFactory.createAnalyzer(session, parameters, bindParameters(statement, parameters), stateMachine.getWarningCollector()) .analyze(statement); List columns = analysis.getOutputDescriptor(statement.getQuery()) diff --git a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java index 66f935284b01..41305cab6723 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java @@ -36,7 +36,7 @@ import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.tree.SetProperties.Type.MATERIALIZED_VIEW; import static io.trino.sql.tree.SetProperties.Type.TABLE; @@ -85,7 +85,7 @@ public ListenableFuture execute( session, plannerContext, accessControl, - parameterExtractor(statement, parameters), + bindParameters(statement, parameters), false); setTableProperties(statement, objectName, session, properties); } @@ -97,7 +97,7 @@ else if (statement.getType() == MATERIALIZED_VIEW) { session, plannerContext, accessControl, - parameterExtractor(statement, parameters), + bindParameters(statement, parameters), false); setMaterializedViewProperties(statement, objectName, session, properties); } diff --git a/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java b/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java index 17a34cdc9f6c..5686db6931fa 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java @@ -37,7 +37,7 @@ import static io.trino.metadata.SessionPropertyManager.evaluatePropertyValue; import static io.trino.metadata.SessionPropertyManager.serializeSessionProperty; import static io.trino.spi.StandardErrorCode.INVALID_SESSION_PROPERTY; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static java.lang.String.format; import static java.util.Objects.requireNonNull; @@ -95,7 +95,7 @@ public ListenableFuture execute( Object objectValue; try { - objectValue = evaluatePropertyValue(statement.getValue(), type, session, plannerContext, accessControl, parameterExtractor(statement, parameters)); + objectValue = evaluatePropertyValue(statement.getValue(), type, session, plannerContext, accessControl, bindParameters(statement, parameters)); } catch (TrinoException e) { throw new TrinoException( diff --git a/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java b/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java index a13beb0099ec..f9c48ff35dca 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java @@ -43,7 +43,7 @@ import static io.trino.spi.StandardErrorCode.TYPE_MISMATCH; import static io.trino.spi.type.TimeZoneKey.getTimeZoneKey; import static io.trino.spi.type.TimeZoneKey.getTimeZoneKeyForOffset; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.ExpressionAnalyzer.createConstantAnalyzer; import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; import static io.trino.util.Failures.checkCondition; @@ -91,7 +91,7 @@ private String getTimeZoneId( List parameters, WarningCollector warningCollector) { - Map, Expression> parameterLookup = parameterExtractor(statement, parameters); + Map, Expression> parameterLookup = bindParameters(statement, parameters); ExpressionAnalyzer analyzer = createConstantAnalyzer( plannerContext, accessControl, diff --git a/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java b/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java index 9b6c0edcfdcc..b6d83324da04 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java +++ b/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java @@ -96,7 +96,7 @@ import static io.trino.execution.QueryState.PLANNING; import static io.trino.server.DynamicFilterService.DynamicFiltersStats; import static io.trino.spi.StandardErrorCode.STACK_OVERFLOW; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static java.lang.Thread.currentThread; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; @@ -260,7 +260,7 @@ private static Analysis analyze( Analyzer analyzer = analyzerFactory.createAnalyzer( stateMachine.getSession(), preparedQuery.getParameters(), - parameterExtractor(preparedQuery.getStatement(), preparedQuery.getParameters()), + bindParameters(preparedQuery.getStatement(), preparedQuery.getParameters()), warningCollector); Analysis analysis; try { diff --git a/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java b/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java index 968733ee6bef..77cf8d02f3e9 100644 --- a/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java +++ b/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java @@ -32,7 +32,7 @@ public class ParameterUtils { private ParameterUtils() {} - public static Map, Expression> parameterExtractor(Statement statement, List parameters) + public static Map, Expression> bindParameters(Statement statement, List values) { List parametersList = getParameters(statement).stream() .sorted(Comparator.comparing( @@ -42,7 +42,7 @@ public static Map, Expression> parameterExtractor(Statement s .collect(toImmutableList()); ImmutableMap.Builder, Expression> builder = ImmutableMap.builder(); - Iterator iterator = parameters.iterator(); + Iterator iterator = values.iterator(); for (Parameter parameter : parametersList) { builder.put(NodeRef.of(parameter), iterator.next()); } diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java index d2c959084dea..0c8297bebcb1 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java @@ -43,7 +43,7 @@ import java.util.Optional; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.QueryType.EXPLAIN; import static io.trino.sql.planner.LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED; import static io.trino.sql.planner.planprinter.IoPlanPrinter.textIoPlan; @@ -172,7 +172,7 @@ public Plan getLogicalPlan(Session session, Statement statement, List parameters, WarningCollector warningCollector) { - Analyzer analyzer = analyzerFactory.createAnalyzer(session, parameters, parameterExtractor(statement, parameters), warningCollector); + Analyzer analyzer = analyzerFactory.createAnalyzer(session, parameters, bindParameters(statement, parameters), warningCollector); return analyzer.analyze(statement, EXPLAIN); } diff --git a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java index 39d0538b0a5e..046fe9768508 100644 --- a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java +++ b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java @@ -237,7 +237,7 @@ import static io.trino.connector.CatalogStore.NO_STORED_CATALOGS; import static io.trino.spi.connector.Constraint.alwaysTrue; import static io.trino.spi.connector.DynamicFilter.EMPTY; -import static io.trino.sql.ParameterUtils.parameterExtractor; +import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.ParsingUtil.createParsingOptions; import static io.trino.sql.planner.LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED; import static io.trino.sql.planner.optimizations.PlanNodeSearcher.searchFrom; @@ -1081,7 +1081,7 @@ public Plan createPlan(Session session, @Language("SQL") String sql, List Date: Fri, 4 Nov 2022 14:52:09 -0700 Subject: [PATCH 2/5] Consolidate ParameterExtractor and ParameterUtils --- .../io/trino/execution/AddColumnTask.java | 2 +- .../java/io/trino/execution/CallTask.java | 2 +- .../execution/CreateMaterializedViewTask.java | 2 +- .../io/trino/execution/CreateSchemaTask.java | 2 +- .../io/trino/execution/CreateTableTask.java | 2 +- .../io/trino/execution/CreateViewTask.java | 2 +- .../trino/execution/ParameterExtractor.java | 26 ++++++++++ .../io/trino/execution/SetPropertiesTask.java | 2 +- .../io/trino/execution/SetSessionTask.java | 2 +- .../io/trino/execution/SetTimeZoneTask.java | 2 +- .../io/trino/execution/SqlQueryExecution.java | 2 +- .../java/io/trino/sql/ParameterUtils.java | 51 ------------------- .../io/trino/sql/analyzer/QueryExplainer.java | 2 +- .../io/trino/testing/LocalQueryRunner.java | 2 +- 14 files changed, 38 insertions(+), 63 deletions(-) delete mode 100644 core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java diff --git a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java index 7a259895587b..7a6420ae3499 100644 --- a/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java @@ -37,6 +37,7 @@ import java.util.Map; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.spi.StandardErrorCode.COLUMN_ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.COLUMN_TYPE_UNKNOWN; @@ -44,7 +45,6 @@ import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.spi.StandardErrorCode.TYPE_NOT_FOUND; import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature; import static io.trino.type.UnknownType.UNKNOWN; diff --git a/core/trino-main/src/main/java/io/trino/execution/CallTask.java b/core/trino-main/src/main/java/io/trino/execution/CallTask.java index c63da0c3d066..8d00652ea379 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CallTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CallTask.java @@ -55,6 +55,7 @@ import static com.google.common.base.Throwables.throwIfInstanceOf; import static com.google.common.base.Verify.verify; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.INVALID_ARGUMENTS; @@ -62,7 +63,6 @@ import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.PROCEDURE_CALL_FAILED; import static io.trino.spi.type.TypeUtils.writeNativeValue; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; import static java.util.Arrays.asList; diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java index 0195d3451dab..920d65adb1cb 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java @@ -39,9 +39,9 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.SqlFormatterUtil.getFormattedSql; import static java.util.Objects.requireNonNull; diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java index 28d19b89f205..82093ebae5e0 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateSchemaTask.java @@ -36,13 +36,13 @@ import java.util.Optional; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.checkRoleExists; import static io.trino.metadata.MetadataUtil.createCatalogSchemaName; import static io.trino.metadata.MetadataUtil.createPrincipal; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.SCHEMA_ALREADY_EXISTS; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static java.util.Objects.requireNonNull; diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java index 93ae644eed3f..c7071d53304d 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java @@ -60,6 +60,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS; @@ -73,7 +74,6 @@ import static io.trino.spi.StandardErrorCode.TYPE_NOT_FOUND; import static io.trino.spi.StandardErrorCode.UNSUPPORTED_TABLE_TYPE; import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature; import static io.trino.sql.tree.LikeClause.PropertiesOption.EXCLUDING; diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java index 612316662dd8..524bb00d93ce 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java @@ -35,9 +35,9 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.spi.StandardErrorCode.TABLE_ALREADY_EXISTS; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.SqlFormatterUtil.getFormattedSql; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.tree.CreateView.Security.INVOKER; diff --git a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java index 35c843e106bf..b55c6d64405e 100644 --- a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java +++ b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java @@ -13,12 +13,21 @@ */ package io.trino.execution; +import com.google.common.collect.ImmutableMap; import io.trino.sql.tree.DefaultTraversalVisitor; +import io.trino.sql.tree.Expression; +import io.trino.sql.tree.NodeLocation; +import io.trino.sql.tree.NodeRef; import io.trino.sql.tree.Parameter; import io.trino.sql.tree.Statement; import java.util.ArrayList; +import java.util.Comparator; +import java.util.Iterator; import java.util.List; +import java.util.Map; + +import static com.google.common.collect.ImmutableList.toImmutableList; public final class ParameterExtractor { @@ -36,6 +45,23 @@ public static List getParameters(Statement statement) return parameterExtractingVisitor.getParameters(); } + public static Map, Expression> bindParameters(Statement statement, List values) + { + List parametersList = getParameters(statement).stream() + .sorted(Comparator.comparing( + parameter -> parameter.getLocation().get(), + Comparator.comparing(NodeLocation::getLineNumber) + .thenComparing(NodeLocation::getColumnNumber))) + .collect(toImmutableList()); + + ImmutableMap.Builder, Expression> builder = ImmutableMap.builder(); + Iterator iterator = values.iterator(); + for (Parameter parameter : parametersList) { + builder.put(NodeRef.of(parameter), iterator.next()); + } + return builder.buildOrThrow(); + } + private static class ParameterExtractingVisitor extends DefaultTraversalVisitor { diff --git a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java index 41305cab6723..6928ffea8897 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java @@ -32,11 +32,11 @@ import java.util.Optional; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.createQualifiedObjectName; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.tree.SetProperties.Type.MATERIALIZED_VIEW; import static io.trino.sql.tree.SetProperties.Type.TABLE; diff --git a/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java b/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java index 5686db6931fa..1583b3830fce 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetSessionTask.java @@ -33,11 +33,11 @@ import java.util.List; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle; import static io.trino.metadata.SessionPropertyManager.evaluatePropertyValue; import static io.trino.metadata.SessionPropertyManager.serializeSessionProperty; import static io.trino.spi.StandardErrorCode.INVALID_SESSION_PROPERTY; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static java.lang.String.format; import static java.util.Objects.requireNonNull; diff --git a/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java b/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java index f9c48ff35dca..33fdba9fbf03 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetTimeZoneTask.java @@ -39,11 +39,11 @@ import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static io.trino.SystemSessionProperties.TIME_ZONE_ID; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.spi.StandardErrorCode.INVALID_LITERAL; import static io.trino.spi.StandardErrorCode.TYPE_MISMATCH; import static io.trino.spi.type.TimeZoneKey.getTimeZoneKey; import static io.trino.spi.type.TimeZoneKey.getTimeZoneKeyForOffset; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.ExpressionAnalyzer.createConstantAnalyzer; import static io.trino.sql.planner.ExpressionInterpreter.evaluateConstantExpression; import static io.trino.util.Failures.checkCondition; diff --git a/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java b/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java index b6d83324da04..d54643f1a5e0 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java +++ b/core/trino-main/src/main/java/io/trino/execution/SqlQueryExecution.java @@ -92,11 +92,11 @@ import static io.trino.SystemSessionProperties.getTaskRetryAttemptsPerTask; import static io.trino.SystemSessionProperties.isEnableDynamicFiltering; import static io.trino.SystemSessionProperties.isFaultTolerantExecutionEventDriverSchedulerEnabled; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.execution.QueryState.FAILED; import static io.trino.execution.QueryState.PLANNING; import static io.trino.server.DynamicFilterService.DynamicFiltersStats; import static io.trino.spi.StandardErrorCode.STACK_OVERFLOW; -import static io.trino.sql.ParameterUtils.bindParameters; import static java.lang.Thread.currentThread; import static java.util.Objects.requireNonNull; import static java.util.concurrent.TimeUnit.SECONDS; diff --git a/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java b/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java deleted file mode 100644 index 77cf8d02f3e9..000000000000 --- a/core/trino-main/src/main/java/io/trino/sql/ParameterUtils.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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 io.trino.sql; - -import com.google.common.collect.ImmutableMap; -import io.trino.sql.tree.Expression; -import io.trino.sql.tree.NodeLocation; -import io.trino.sql.tree.NodeRef; -import io.trino.sql.tree.Parameter; -import io.trino.sql.tree.Statement; - -import java.util.Comparator; -import java.util.Iterator; -import java.util.List; -import java.util.Map; - -import static com.google.common.collect.ImmutableList.toImmutableList; -import static io.trino.execution.ParameterExtractor.getParameters; - -public class ParameterUtils -{ - private ParameterUtils() {} - - public static Map, Expression> bindParameters(Statement statement, List values) - { - List parametersList = getParameters(statement).stream() - .sorted(Comparator.comparing( - parameter -> parameter.getLocation().get(), - Comparator.comparing(NodeLocation::getLineNumber) - .thenComparing(NodeLocation::getColumnNumber))) - .collect(toImmutableList()); - - ImmutableMap.Builder, Expression> builder = ImmutableMap.builder(); - Iterator iterator = values.iterator(); - for (Parameter parameter : parametersList) { - builder.put(NodeRef.of(parameter), iterator.next()); - } - return builder.buildOrThrow(); - } -} diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java index 0c8297bebcb1..9cb5ce07098f 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/QueryExplainer.java @@ -42,8 +42,8 @@ import java.util.List; import java.util.Optional; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.analyzer.QueryType.EXPLAIN; import static io.trino.sql.planner.LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED; import static io.trino.sql.planner.planprinter.IoPlanPrinter.textIoPlan; diff --git a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java index 046fe9768508..48a9c8ed3617 100644 --- a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java +++ b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java @@ -235,9 +235,9 @@ import static io.trino.connector.CatalogServiceProviderModule.createTableProceduresProvider; import static io.trino.connector.CatalogServiceProviderModule.createTablePropertyManager; import static io.trino.connector.CatalogStore.NO_STORED_CATALOGS; +import static io.trino.execution.ParameterExtractor.bindParameters; import static io.trino.spi.connector.Constraint.alwaysTrue; import static io.trino.spi.connector.DynamicFilter.EMPTY; -import static io.trino.sql.ParameterUtils.bindParameters; import static io.trino.sql.ParsingUtil.createParsingOptions; import static io.trino.sql.planner.LogicalPlanner.Stage.OPTIMIZED_AND_VALIDATED; import static io.trino.sql.planner.optimizations.PlanNodeSearcher.searchFrom; From 412f27cda8790af849433cefd00c5aa6fd4ead78 Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Fri, 4 Nov 2022 14:52:49 -0700 Subject: [PATCH 3/5] Rename getParameters to extractParameters --- .../main/java/io/trino/execution/ParameterExtractor.java | 6 +++--- .../java/io/trino/sql/rewrite/DescribeInputRewrite.java | 4 ++-- .../java/io/trino/execution/TestParameterExtractor.java | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java index b55c6d64405e..a3388c0db5dc 100644 --- a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java +++ b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java @@ -35,10 +35,10 @@ private ParameterExtractor() {} public static int getParameterCount(Statement statement) { - return getParameters(statement).size(); + return extractParameters(statement).size(); } - public static List getParameters(Statement statement) + public static List extractParameters(Statement statement) { ParameterExtractingVisitor parameterExtractingVisitor = new ParameterExtractingVisitor(); parameterExtractingVisitor.process(statement, null); @@ -47,7 +47,7 @@ public static List getParameters(Statement statement) public static Map, Expression> bindParameters(Statement statement, List values) { - List parametersList = getParameters(statement).stream() + List parametersList = extractParameters(statement).stream() .sorted(Comparator.comparing( parameter -> parameter.getLocation().get(), Comparator.comparing(NodeLocation::getLineNumber) diff --git a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java index 52ae3ad9a630..70db47fb319e 100644 --- a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java +++ b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java @@ -41,7 +41,7 @@ import java.util.Optional; import static io.trino.SystemSessionProperties.isOmitDateTimeTypePrecision; -import static io.trino.execution.ParameterExtractor.getParameters; +import static io.trino.execution.ParameterExtractor.extractParameters; import static io.trino.sql.ParsingUtil.createParsingOptions; import static io.trino.sql.QueryUtil.aliased; import static io.trino.sql.QueryUtil.ascending; @@ -116,7 +116,7 @@ protected Node visitDescribeInput(DescribeInput node, Void context) Analysis analysis = analyzer.analyze(statement, DESCRIBE); // get all parameters in query - List parameters = getParameters(statement); + List parameters = extractParameters(statement); // return the positions and types of all parameters Row[] rows = parameters.stream().map(parameter -> createDescribeInputRow(session, parameter, analysis)).toArray(Row[]::new); diff --git a/core/trino-main/src/test/java/io/trino/execution/TestParameterExtractor.java b/core/trino-main/src/test/java/io/trino/execution/TestParameterExtractor.java index 3ae15f0ba053..70cdc9711625 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestParameterExtractor.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestParameterExtractor.java @@ -30,7 +30,7 @@ public class TestParameterExtractor public void testNoParameter() { Statement statement = sqlParser.createStatement("SELECT c1, c2 FROM test_table WHERE c1 = 1 AND c2 > 2", new ParsingOptions()); - assertThat(ParameterExtractor.getParameters(statement)).isEmpty(); + assertThat(ParameterExtractor.extractParameters(statement)).isEmpty(); assertThat(ParameterExtractor.getParameterCount(statement)).isEqualTo(0); } @@ -38,7 +38,7 @@ public void testNoParameter() public void testParameterCount() { Statement statement = sqlParser.createStatement("SELECT c1, c2 FROM test_table WHERE c1 = ? AND c2 > ?", new ParsingOptions()); - assertThat(ParameterExtractor.getParameters(statement)) + assertThat(ParameterExtractor.extractParameters(statement)) .containsExactly( new Parameter(new NodeLocation(1, 41), 0), new Parameter(new NodeLocation(1, 52), 1)); @@ -49,7 +49,7 @@ public void testParameterCount() public void testShowStats() { Statement statement = sqlParser.createStatement("SHOW STATS FOR (SELECT c1, c2 FROM test_table WHERE c1 = ? AND c2 > ?)", new ParsingOptions()); - assertThat(ParameterExtractor.getParameters(statement)) + assertThat(ParameterExtractor.extractParameters(statement)) .containsExactly( new Parameter(new NodeLocation(1, 57), 0), new Parameter(new NodeLocation(1, 68), 1)); @@ -60,7 +60,7 @@ public void testShowStats() public void testLambda() { Statement statement = sqlParser.createStatement("SELECT * FROM test_table WHERE any_match(items, x -> x > ?)", new ParsingOptions()); - assertThat(ParameterExtractor.getParameters(statement)) + assertThat(ParameterExtractor.extractParameters(statement)) .containsExactly(new Parameter(new NodeLocation(1, 58), 0)); assertThat(ParameterExtractor.getParameterCount(statement)).isEqualTo(1); From 60df58a67839ad43fab7fac6a57236232ebc5b71 Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Fri, 4 Nov 2022 15:10:56 -0700 Subject: [PATCH 4/5] Rename parameter "position" to "id" The position does not actually match the order in which they are supposed to be processed. It's needed just to assign a unique identity to each parameter node. --- .../trino/sql/analyzer/AggregationAnalyzer.java | 2 +- .../trino/sql/analyzer/ExpressionAnalyzer.java | 4 ++-- .../io/trino/sql/planner/ParameterRewriter.java | 2 +- .../io/trino/sql/planner/TranslationMap.java | 2 +- .../trino/sql/rewrite/DescribeInputRewrite.java | 2 +- .../main/java/io/trino/sql/tree/Parameter.java | 16 ++++++++-------- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/AggregationAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/AggregationAnalyzer.java index c56f7dc1fe56..bc360571822e 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/AggregationAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/AggregationAnalyzer.java @@ -694,7 +694,7 @@ protected Boolean visitParameter(Parameter node, Void context) return true; } Map, Expression> parameters = analysis.getParameters(); - checkArgument(node.getPosition() < parameters.size(), "Invalid parameter number %s, max values is %s", node.getPosition(), parameters.size() - 1); + checkArgument(node.getId() < parameters.size(), "Invalid parameter number %s, max values is %s", node.getId(), parameters.size() - 1); return process(parameters.get(NodeRef.of(node)), context); } diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java index 7217a9ed1620..4a4924604e45 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java @@ -2070,8 +2070,8 @@ protected Type visitParameter(Parameter node, StackableAstVisitorContext= parameters.size()) { - throw semanticException(INVALID_PARAMETER_USAGE, node, "Invalid parameter index %s, max value is %s", node.getPosition(), parameters.size() - 1); + if (node.getId() >= parameters.size()) { + throw semanticException(INVALID_PARAMETER_USAGE, node, "Invalid parameter index %s, max value is %s", node.getId(), parameters.size() - 1); } Expression providedValue = parameters.get(NodeRef.of(node)); diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/ParameterRewriter.java b/core/trino-main/src/main/java/io/trino/sql/planner/ParameterRewriter.java index a9ac38eb0fd1..596fbd99a659 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/ParameterRewriter.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/ParameterRewriter.java @@ -56,7 +56,7 @@ protected Expression rewriteExpression(Expression node, Void context, Expression @Override public Expression rewriteParameter(Parameter node, Void context, ExpressionTreeRewriter treeRewriter) { - checkState(parameters.size() > node.getPosition(), "Too few parameter values"); + checkState(parameters.size() > node.getId(), "Too few parameter values"); return coerceIfNecessary(node, parameters.get(NodeRef.of(node))); } diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/TranslationMap.java b/core/trino-main/src/main/java/io/trino/sql/planner/TranslationMap.java index 66661a3a79be..d9509d02303d 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/TranslationMap.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/TranslationMap.java @@ -404,7 +404,7 @@ public Expression rewriteParameter(Parameter node, Void context, ExpressionTreeR return coerceIfNecessary(node, mapped.get()); } - checkState(analysis.getParameters().size() > node.getPosition(), "Too few parameter values"); + checkState(analysis.getParameters().size() > node.getId(), "Too few parameter values"); return coerceIfNecessary(node, treeRewriter.rewrite(analysis.getParameters().get(NodeRef.of(node)), null)); } diff --git a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java index 70db47fb319e..df698100abbe 100644 --- a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java +++ b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java @@ -148,7 +148,7 @@ private static Row createDescribeInputRow(Session session, Parameter parameter, } return row( - new LongLiteral(Integer.toString(parameter.getPosition())), + new LongLiteral(Integer.toString(parameter.getId())), new StringLiteral(getDisplayLabel(type, isOmitDateTimeTypePrecision(session)))); } diff --git a/core/trino-parser/src/main/java/io/trino/sql/tree/Parameter.java b/core/trino-parser/src/main/java/io/trino/sql/tree/Parameter.java index b12e31ac7417..9b88948fa16a 100644 --- a/core/trino-parser/src/main/java/io/trino/sql/tree/Parameter.java +++ b/core/trino-parser/src/main/java/io/trino/sql/tree/Parameter.java @@ -22,7 +22,7 @@ public class Parameter extends Expression { - private final int position; + private final int id; public Parameter(int id) { @@ -34,15 +34,15 @@ public Parameter(NodeLocation location, int id) this(Optional.of(location), id); } - private Parameter(Optional location, int position) + private Parameter(Optional location, int id) { super(location); - this.position = position; + this.id = id; } - public int getPosition() + public int getId() { - return position; + return id; } @Override @@ -68,13 +68,13 @@ public boolean equals(Object o) } Parameter that = (Parameter) o; - return Objects.equals(position, that.position); + return Objects.equals(id, that.id); } @Override public int hashCode() { - return position; + return id; } @Override @@ -84,6 +84,6 @@ public boolean shallowEquals(Node other) return false; } - return position == ((Parameter) other).position; + return id == ((Parameter) other).id; } } From a2e9d4754d0f7262e4746c9ddd6e30ec69742901 Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Fri, 4 Nov 2022 15:16:24 -0700 Subject: [PATCH 5/5] Fix incorrect order of parameters in DESCRIBE INPUT They were being listed in depth-first-search order instead of in the order in which they appear in the query text. --- .../trino/execution/ParameterExtractor.java | 12 ++++----- .../sql/rewrite/DescribeInputRewrite.java | 11 +++++--- .../AbstractTestEngineOnlyQueries.java | 25 +++++++++++++++++++ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java index a3388c0db5dc..345b959317a0 100644 --- a/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java +++ b/core/trino-main/src/main/java/io/trino/execution/ParameterExtractor.java @@ -42,17 +42,17 @@ public static List extractParameters(Statement statement) { ParameterExtractingVisitor parameterExtractingVisitor = new ParameterExtractingVisitor(); parameterExtractingVisitor.process(statement, null); - return parameterExtractingVisitor.getParameters(); - } - - public static Map, Expression> bindParameters(Statement statement, List values) - { - List parametersList = extractParameters(statement).stream() + return parameterExtractingVisitor.getParameters().stream() .sorted(Comparator.comparing( parameter -> parameter.getLocation().get(), Comparator.comparing(NodeLocation::getLineNumber) .thenComparing(NodeLocation::getColumnNumber))) .collect(toImmutableList()); + } + + public static Map, Expression> bindParameters(Statement statement, List values) + { + List parametersList = extractParameters(statement); ImmutableMap.Builder, Expression> builder = ImmutableMap.builder(); Iterator iterator = values.iterator(); diff --git a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java index df698100abbe..7dbd0e183d15 100644 --- a/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java +++ b/core/trino-main/src/main/java/io/trino/sql/rewrite/DescribeInputRewrite.java @@ -118,8 +118,13 @@ protected Node visitDescribeInput(DescribeInput node, Void context) // get all parameters in query List parameters = extractParameters(statement); + ImmutableList.Builder builder = ImmutableList.builder(); + for (int i = 0; i < parameters.size(); i++) { + builder.add(createDescribeInputRow(session, i, parameters.get(i), analysis)); + } + // return the positions and types of all parameters - Row[] rows = parameters.stream().map(parameter -> createDescribeInputRow(session, parameter, analysis)).toArray(Row[]::new); + Row[] rows = builder.build().toArray(Row[]::new); Optional limit = Optional.empty(); if (rows.length == 0) { rows = new Row[] {row(new NullLiteral(), new NullLiteral())}; @@ -140,7 +145,7 @@ protected Node visitDescribeInput(DescribeInput node, Void context) limit); } - private static Row createDescribeInputRow(Session session, Parameter parameter, Analysis queryAnalysis) + private static Row createDescribeInputRow(Session session, int position, Parameter parameter, Analysis queryAnalysis) { Type type = queryAnalysis.getCoercion(parameter); if (type == null) { @@ -148,7 +153,7 @@ private static Row createDescribeInputRow(Session session, Parameter parameter, } return row( - new LongLiteral(Integer.toString(parameter.getId())), + new LongLiteral(Integer.toString(position)), new StringLiteral(getDisplayLabel(type, isOmitDateTimeTypePrecision(session)))); } diff --git a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestEngineOnlyQueries.java b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestEngineOnlyQueries.java index 09fd81a5137d..88d6fd7bfff1 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestEngineOnlyQueries.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/AbstractTestEngineOnlyQueries.java @@ -1366,6 +1366,31 @@ public void testDescribeInput() assertEqualsIgnoreOrder(actual, expected); } + @Test + public void testDescribeInputWithClause() + { + Session session = Session.builder(getSession()) + .addPreparedStatement("my_query", """ + WITH t2 AS ( + SELECT * FROM (VALUES 1) AS t2(b) + WHERE b = ?) + SELECT * + FROM + (VALUES ('', 1)) AS t1(a, b) + JOIN t2 ON t2.b = t1.b + WHERE t1.a = ? + """) + .build(); + + MaterializedResult actual = computeActual(session, "DESCRIBE INPUT my_query"); + MaterializedResult expected = resultBuilder(session, BIGINT, VARCHAR) + .row(0, "integer") + .row(1, "varchar(0)") + .build(); + + assertEqualsIgnoreOrder(actual, expected); + } + @Test public void testDescribeInputWithAggregation() {