From d1e070c428e02dccad5f6b8068df14048c1b1591 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 8 Nov 2022 10:30:37 +0100 Subject: [PATCH 1/5] Fix a typo in comment --- .../src/main/java/io/trino/testng/services/Listeners.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java b/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java index 553b0fc782a9..80387347f591 100644 --- a/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java +++ b/testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java @@ -24,7 +24,7 @@ private Listeners() {} /** * Print error to standard error and exit JVM. * - * @apiNote A TestNG listener cannot throw an exception, as this are not currently properly handlded by TestNG. + * @apiNote A TestNG listener cannot throw an exception, as this are not currently properly handled by TestNG. */ public static void reportListenerFailure(Class listenerClass, String format, Object... args) { From c4eb2e59c0cd5358e434dad28552b10f06a8ca43 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 8 Nov 2022 11:06:53 +0100 Subject: [PATCH 2/5] Call semanticException method as a format method in CommentTask --- .../java/io/trino/execution/CommentTask.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/execution/CommentTask.java b/core/trino-main/src/main/java/io/trino/execution/CommentTask.java index 3448089fd3bc..80f705b4ddb5 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CommentTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CommentTask.java @@ -40,7 +40,6 @@ import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.sql.analyzer.SemanticExceptions.semanticException; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class CommentTask @@ -118,14 +117,17 @@ private void commentOnView(Comment statement, Session session) { QualifiedObjectName viewName = createQualifiedObjectName(session, statement, statement.getName()); if (metadata.getView(session, viewName).isEmpty()) { - String exceptionMessage = format("View '%s' does not exist", viewName); + String additionalInformation; if (metadata.getMaterializedView(session, viewName).isPresent()) { - exceptionMessage += ", but a materialized view with that name exists. Setting comments on materialized views is unsupported."; + additionalInformation = ", but a materialized view with that name exists. Setting comments on materialized views is unsupported."; } else if (metadata.getTableHandle(session, viewName).isPresent()) { - exceptionMessage += ", but a table with that name exists. Did you mean COMMENT ON TABLE " + viewName + " IS ...?"; + additionalInformation = ", but a table with that name exists. Did you mean COMMENT ON TABLE " + viewName + " IS ...?"; } - throw semanticException(TABLE_NOT_FOUND, statement, exceptionMessage); + else { + additionalInformation = ""; + } + throw semanticException(TABLE_NOT_FOUND, statement, "View '%s' does not exist%s", viewName, additionalInformation); } accessControl.checkCanSetViewComment(session.toSecurityContext(), viewName); @@ -144,7 +146,7 @@ private void commentOnColumn(Comment statement, Session session) ViewColumn viewColumn = viewDefinition.getColumns().stream() .filter(column -> column.getName().equals(columnName)) .findAny() - .orElseThrow(() -> semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName)); + .orElseThrow(() -> semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: %s", columnName)); accessControl.checkCanSetColumnComment(session.toSecurityContext(), originalObjectName); metadata.setViewColumnComment(session, originalObjectName, viewColumn.getName(), statement.getComment()); @@ -155,14 +157,14 @@ else if (metadata.isMaterializedView(session, originalObjectName)) { else { RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalObjectName); if (redirectionAwareTableHandle.getTableHandle().isEmpty()) { - throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + originalObjectName); + throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: %s", originalObjectName); } TableHandle tableHandle = redirectionAwareTableHandle.getTableHandle().get(); String columnName = statement.getName().getSuffix(); Map columnHandles = metadata.getColumnHandles(session, tableHandle); if (!columnHandles.containsKey(columnName)) { - throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName); + throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: %s", columnName); } accessControl.checkCanSetColumnComment(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(originalObjectName)); From 8015105456d973ec14abe8bb0ca593c47fbe8bf1 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 8 Nov 2022 11:11:23 +0100 Subject: [PATCH 3/5] Call semanticException method as a format method in CreateTableTask --- .../java/io/trino/execution/CreateTableTask.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 c7071d53304d..961013ec4af6 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 @@ -79,7 +79,6 @@ import static io.trino.sql.tree.LikeClause.PropertiesOption.EXCLUDING; import static io.trino.sql.tree.LikeClause.PropertiesOption.INCLUDING; import static io.trino.type.UnknownType.UNKNOWN; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class CreateTableTask @@ -201,11 +200,18 @@ else if (element instanceof LikeClause) { LikeClause.PropertiesOption propertiesOption = likeClause.getPropertiesOption().orElse(EXCLUDING); QualifiedObjectName likeTableName = redirection.getRedirectedTableName().orElse(originalLikeTableName); if (propertiesOption == INCLUDING && !catalogName.equals(likeTableName.getCatalogName())) { - String message = "CREATE TABLE LIKE table INCLUDING PROPERTIES across catalogs is not supported"; if (!originalLikeTableName.equals(likeTableName)) { - message += format(". LIKE table '%s' redirected to '%s'.", originalLikeTableName, likeTableName); + throw semanticException( + NOT_SUPPORTED, + statement, + "CREATE TABLE LIKE table INCLUDING PROPERTIES across catalogs is not supported. LIKE table '%s' redirected to '%s'.", + originalLikeTableName, + likeTableName); } - throw semanticException(NOT_SUPPORTED, statement, message); + throw semanticException( + NOT_SUPPORTED, + statement, + "CREATE TABLE LIKE table INCLUDING PROPERTIES across catalogs is not supported"); } TableMetadata likeTableMetadata = plannerContext.getMetadata().getTableMetadata(session, likeTable); From 556fc7206e88b80865a2d43bd176d7ebf7de0cd6 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 8 Nov 2022 11:14:04 +0100 Subject: [PATCH 4/5] Call semanticException method as a format method in SetPropertiesTask --- .../java/io/trino/execution/SetPropertiesTask.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 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 6928ffea8897..0121b5809a4c 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 @@ -40,7 +40,6 @@ 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; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class SetPropertiesTask @@ -132,14 +131,17 @@ private void setMaterializedViewProperties( Map> properties) { if (plannerContext.getMetadata().getMaterializedView(session, materializedViewName).isEmpty()) { - String exceptionMessage = format("Materialized View '%s' does not exist", materializedViewName); + String additionalInformation; if (plannerContext.getMetadata().getView(session, materializedViewName).isPresent()) { - exceptionMessage += ", but a view with that name exists."; + additionalInformation = ", but a view with that name exists."; } else if (plannerContext.getMetadata().getTableHandle(session, materializedViewName).isPresent()) { - exceptionMessage += ", but a table with that name exists. Did you mean ALTER TABLE " + materializedViewName + " SET PROPERTIES ...?"; + additionalInformation = ", but a table with that name exists. Did you mean ALTER TABLE " + materializedViewName + " SET PROPERTIES ...?"; } - throw semanticException(TABLE_NOT_FOUND, statement, exceptionMessage); + else { + additionalInformation = ""; + } + throw semanticException(TABLE_NOT_FOUND, statement, "Materialized View '%s' does not exist%s", materializedViewName, additionalInformation); } accessControl.checkCanSetMaterializedViewProperties(session.toSecurityContext(), materializedViewName, properties); plannerContext.getMetadata().setMaterializedViewProperties(session, materializedViewName, properties); From 73db9867f336c999fdbed433e8c7db00a5723f17 Mon Sep 17 00:00:00 2001 From: Krzysztof Sobolewski Date: Tue, 8 Nov 2022 11:34:30 +0100 Subject: [PATCH 5/5] Call semanticException method as a format method in StatementAnalyzer --- .../trino/sql/analyzer/StatementAnalyzer.java | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 3b5ec1b24c5d..011984459634 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -677,9 +677,12 @@ protected Scope visitRefreshMaterializedView(RefreshMaterializedView refreshMate .collect(toImmutableList()); if (!typesMatchForInsert(tableTypes, queryTypes)) { - throw semanticException(TYPE_MISMATCH, refreshMaterializedView, "Insert query has mismatched column types: " + - "Table: [" + Joiner.on(", ").join(tableTypes) + "], " + - "Query: [" + Joiner.on(", ").join(queryTypes) + "]"); + throw semanticException( + TYPE_MISMATCH, + refreshMaterializedView, + "Insert query has mismatched column types: Table: [%s], Query: [%s]", + Joiner.on(", ").join(tableTypes), + Joiner.on(", ").join(queryTypes)); } Stream columns = Streams.zip( @@ -1136,7 +1139,7 @@ protected Scope visitTableExecute(TableExecute node, Optional scope) // analyze WHERE if (!procedureMetadata.getExecutionMode().supportsFilter() && node.getWhere().isPresent()) { - throw semanticException(NOT_SUPPORTED, node, "WHERE not supported for procedure " + procedureName); + throw semanticException(NOT_SUPPORTED, node, "WHERE not supported for procedure %s", procedureName); } node.getWhere().ifPresent(where -> analyzeWhere(node, tableScope, where)); @@ -1546,7 +1549,7 @@ else if (returnTypeSpecification == GENERIC_TABLE) { // table alias is mandatory for a polymorphic table function invocation which produces proper columns. // We don't enforce this requirement. properColumnsDescriptor = analyzedProperColumnsDescriptor - .orElseThrow(() -> semanticException(MISSING_RETURN_TYPE, node, "Cannot determine returned relation type for table function " + node.getName())); + .orElseThrow(() -> semanticException(MISSING_RETURN_TYPE, node, "Cannot determine returned relation type for table function %s", node.getName())); } else { // returned type is statically declared at function declaration // According to SQL standard ISO/IEC 9075-2, 7.6 , p. 409, @@ -1660,11 +1663,11 @@ private ArgumentsAnalysis analyzeArguments(List argumentS for (TableFunctionArgument argument : arguments) { String argumentName = argument.getName().orElseThrow().getCanonicalValue(); if (!uniqueArgumentNames.add(argumentName)) { - throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Duplicate argument name: " + argumentName); + throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Duplicate argument name: %s", argumentName); } ArgumentSpecification argumentSpecification = argumentSpecificationsByName.remove(argumentName); if (argumentSpecification == null) { - throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Unexpected argument name: " + argumentName); + throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Unexpected argument name: %s", argumentName); } ArgumentAnalysis argumentAnalysis = analyzeArgument(argumentSpecification, argument, scope); passedArguments.put(argumentSpecification.getName(), argumentAnalysis.getArgument()); @@ -1707,7 +1710,7 @@ else if (argument.getValue() instanceof Expression) { actualType = "expression"; } else { - throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Unexpected table function argument type: " + argument.getClass().getSimpleName()); + throw semanticException(INVALID_FUNCTION_ARGUMENT, argument, "Unexpected table function argument type: %s", argument.getClass().getSimpleName()); } if (argumentSpecification instanceof TableArgumentSpecification) { @@ -1879,7 +1882,7 @@ public Expression rewriteParameter(Parameter node, Void context, ExpressionTreeR private Argument analyzeDefault(ArgumentSpecification argumentSpecification, Node errorLocation) { if (argumentSpecification.isRequired()) { - throw semanticException(MISSING_ARGUMENT, errorLocation, "Missing argument: " + argumentSpecification.getName()); + throw semanticException(MISSING_ARGUMENT, errorLocation, "Missing argument: %s", argumentSpecification.getName()); } checkArgument(!(argumentSpecification instanceof TableArgumentSpecification), "invalid table argument specification: default set"); @@ -1939,10 +1942,10 @@ else if (name.getParts().size() == 3) { candidates = qualifiedInputs.get(QualifiedName.of(fullyQualifiedName.getCatalogName(), fullyQualifiedName.getSchemaName(), fullyQualifiedName.getObjectName())); } if (candidates.isEmpty()) { - throw semanticException(INVALID_COPARTITIONING, name.getOriginalParts().get(0), "No table argument found for name: " + name); + throw semanticException(INVALID_COPARTITIONING, name.getOriginalParts().get(0), "No table argument found for name: %s", name); } if (candidates.size() > 1) { - throw semanticException(INVALID_COPARTITIONING, name.getOriginalParts().get(0), "Ambiguous reference: multiple table arguments found for name: " + name); + throw semanticException(INVALID_COPARTITIONING, name.getOriginalParts().get(0), "Ambiguous reference: multiple table arguments found for name: %s", name); } TableArgumentAnalysis argument = getOnlyElement(candidates); if (!referencedArguments.add(argument.getArgumentName())) { @@ -2549,7 +2552,7 @@ else if (expression instanceof DereferenceExpression) { qualifiedName = getQualifiedName((DereferenceExpression) expression); } else { - throw semanticException(INVALID_COLUMN_REFERENCE, expression, "Expected column reference. Actual: " + expression); + throw semanticException(INVALID_COLUMN_REFERENCE, expression, "Expected column reference. Actual: %s", expression); } Optional field = inputScope.tryResolveField(expression, qualifiedName); if (field.isEmpty() || !field.get().isLocal()) { @@ -2718,7 +2721,7 @@ private RelationType aliasTableFunctionInvocation(AliasedRelation relation, Rela .map(name -> name.toLowerCase(ENGLISH)) .forEach(name -> { if (!names.add(name)) { - throw semanticException(DUPLICATE_COLUMN_NAME, relation.getRelation(), "Duplicate name of table function proper column: " + name); + throw semanticException(DUPLICATE_COLUMN_NAME, relation.getRelation(), "Duplicate name of table function proper column: %s", name); } }); @@ -2949,12 +2952,13 @@ protected Scope visitSetOperation(SetOperation node, Optional scope) if (node instanceof Intersect || node instanceof Except || node instanceof Union && node.isDistinct()) { for (Type type : outputFieldTypes) { if (!type.isComparable()) { - StringBuilder message = new StringBuilder(format("Type %s is not comparable and therefore cannot be used in ", type)); - message.append(setOperationName); - if (node instanceof Union) { - message.append(" DISTINCT"); - } - throw semanticException(TYPE_MISMATCH, node, message.toString()); + throw semanticException( + TYPE_MISMATCH, + node, + "Type %s is not comparable and therefore cannot be used in %s%s", + type, + setOperationName, + node instanceof Union ? " DISTINCT" : ""); } } } @@ -3619,7 +3623,7 @@ private ResolvedWindow resolveWindowSpecification(QuerySpecification querySpecif CanonicalizationAware canonicalName = canonicalizationAwareKey(windowReference.getName()); ResolvedWindow referencedWindow = analysis.getWindowDefinition(querySpecification, canonicalName); if (referencedWindow == null) { - throw semanticException(INVALID_WINDOW_REFERENCE, windowReference.getName(), "Cannot resolve WINDOW name " + windowReference.getName()); + throw semanticException(INVALID_WINDOW_REFERENCE, windowReference.getName(), "Cannot resolve WINDOW name %s", windowReference.getName()); } return new ResolvedWindow( @@ -3638,7 +3642,7 @@ private ResolvedWindow resolveWindowSpecification(QuerySpecification querySpecif CanonicalizationAware canonicalName = canonicalizationAwareKey(referencedName); ResolvedWindow referencedWindow = analysis.getWindowDefinition(querySpecification, canonicalName); if (referencedWindow == null) { - throw semanticException(INVALID_WINDOW_REFERENCE, referencedName, "Cannot resolve WINDOW name " + referencedName); + throw semanticException(INVALID_WINDOW_REFERENCE, referencedName, "Cannot resolve WINDOW name %s", referencedName); } // analyze dependencies between this window specification and referenced window specification @@ -5226,9 +5230,9 @@ private void validateVersionPointer(QueryPeriod queryPeriod, TableVersion extrac if (!(type instanceof TimestampWithTimeZoneType || type instanceof TimestampType || type instanceof DateType)) { - throw semanticException(TYPE_MISMATCH, queryPeriod, format( + throw semanticException(TYPE_MISMATCH, queryPeriod, "Type %s invalid. Temporal pointers must be of type Timestamp, Timestamp with Time Zone, or Date.", - type.getDisplayName())); + type.getDisplayName()); } if (pointer == null) { throw semanticException(INVALID_ARGUMENTS, queryPeriod, "Pointer value cannot be NULL"); @@ -5239,7 +5243,8 @@ private void validateVersionPointer(QueryPeriod queryPeriod, TableVersion extrac throw semanticException( INVALID_ARGUMENTS, queryPeriod, - format("Pointer value '%s' is not in the past", varchar)); + "Pointer value '%s' is not in the past", + varchar); } } else {