diff --git a/core/trino-main/src/main/java/io/trino/Session.java b/core/trino-main/src/main/java/io/trino/Session.java index 0fd1b26ce096..0052b13d277c 100644 --- a/core/trino-main/src/main/java/io/trino/Session.java +++ b/core/trino-main/src/main/java/io/trino/Session.java @@ -279,7 +279,7 @@ public String getPreparedStatementFromExecute(Execute execute) public String getPreparedStatement(String name) { String sql = preparedStatements.get(name); - checkCondition(sql != null, NOT_FOUND, "Prepared statement not found: " + name); + checkCondition(sql != null, NOT_FOUND, "Prepared statement not found: %s", name); return sql; } diff --git a/core/trino-main/src/main/java/io/trino/execution/SetColumnTypeTask.java b/core/trino-main/src/main/java/io/trino/execution/SetColumnTypeTask.java index 196759a13080..dd5a90d26c2d 100644 --- a/core/trino-main/src/main/java/io/trino/execution/SetColumnTypeTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/SetColumnTypeTask.java @@ -42,7 +42,6 @@ import static io.trino.sql.analyzer.SemanticExceptions.semanticException; import static io.trino.sql.analyzer.TypeSignatureTranslator.toTypeSignature; import static io.trino.type.UnknownType.UNKNOWN; -import static java.lang.String.format; import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; @@ -78,15 +77,18 @@ public ListenableFuture execute( QualifiedObjectName qualifiedObjectName = createQualifiedObjectName(session, statement, statement.getTableName()); RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, qualifiedObjectName); if (redirectionAwareTableHandle.getTableHandle().isEmpty()) { - String exceptionMessage = format("Table '%s' does not exist", qualifiedObjectName); + String additionalInformation; if (metadata.getMaterializedView(session, qualifiedObjectName).isPresent()) { - exceptionMessage += ", but a materialized view with that name exists."; + additionalInformation = ", but a materialized view with that name exists."; } else if (metadata.getView(session, qualifiedObjectName).isPresent()) { - exceptionMessage += ", but a view with that name exists."; + additionalInformation = ", but a view with that name exists."; + } + else { + additionalInformation = ""; } if (!statement.isTableExists()) { - throw semanticException(TABLE_NOT_FOUND, statement, exceptionMessage); + throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist%s", qualifiedObjectName, additionalInformation); } return immediateVoidFuture(); } 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 9666dfe9e291..d7f24294b550 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 @@ -110,7 +110,7 @@ public ListenableFuture execute( propertyMetadata.decode(objectValue); } catch (RuntimeException e) { - throw semanticException(INVALID_SESSION_PROPERTY, statement, e.getMessage()); + throw semanticException(INVALID_SESSION_PROPERTY, statement, "Invalid session property value '%s': %s", propertyName.toString(), e.getMessage()); } stateMachine.addSetSessionProperties(propertyName.toString(), value); diff --git a/core/trino-main/src/main/java/io/trino/json/PathEvaluationError.java b/core/trino-main/src/main/java/io/trino/json/PathEvaluationError.java index 4cf246f43958..2423a21011bf 100644 --- a/core/trino-main/src/main/java/io/trino/json/PathEvaluationError.java +++ b/core/trino-main/src/main/java/io/trino/json/PathEvaluationError.java @@ -13,6 +13,7 @@ */ package io.trino.json; +import com.google.errorprone.annotations.FormatMethod; import io.trino.spi.TrinoException; import static io.trino.spi.StandardErrorCode.PATH_EVALUATION_ERROR; @@ -44,6 +45,7 @@ public PathEvaluationError(Throwable cause) * to the chosen `ON ERROR` option. Non-structural errors (e.g. numeric exceptions) * are not suppressed in `lax` or `strict` mode. */ + @FormatMethod public static TrinoException structuralError(String format, Object... arguments) { return new PathEvaluationError("structural error: " + format(format, arguments)); diff --git a/core/trino-main/src/main/java/io/trino/metadata/FunctionManager.java b/core/trino-main/src/main/java/io/trino/metadata/FunctionManager.java index e5a63bc53f66..d86359c5ffd9 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/FunctionManager.java +++ b/core/trino-main/src/main/java/io/trino/metadata/FunctionManager.java @@ -15,6 +15,7 @@ import com.google.common.cache.CacheBuilder; import com.google.common.util.concurrent.UncheckedExecutionException; +import com.google.errorprone.annotations.FormatMethod; import io.trino.FeaturesConfig; import io.trino.collect.cache.NonEvictableCache; import io.trino.connector.CatalogServiceProvider; @@ -249,6 +250,7 @@ private static void verifyMethodHandleSignature(BoundSignature boundSignature, S } } + @FormatMethod private static void verifyFunctionSignature(boolean check, String message, Object... args) { if (!check) { diff --git a/core/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.java b/core/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.java index 1651fb71a69c..92c500b69d20 100644 --- a/core/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.java +++ b/core/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.java @@ -125,7 +125,7 @@ public SpecializedSqlScalarFunction specialize(BoundSignature boundSignature, Fu ParametricScalarImplementation exactImplementation = implementations.getExactImplementations().get(boundSignature.toSignature()); if (exactImplementation != null) { Optional scalarFunctionImplementation = exactImplementation.specialize(functionBinding, functionDependencies); - checkCondition(scalarFunctionImplementation.isPresent(), FUNCTION_IMPLEMENTATION_ERROR, format("Exact implementation of %s do not match expected java types.", boundSignature.getName())); + checkCondition(scalarFunctionImplementation.isPresent(), FUNCTION_IMPLEMENTATION_ERROR, "Exact implementation of %s do not match expected java types.", boundSignature.getName()); return scalarFunctionImplementation.get(); } diff --git a/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java b/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java index f8c84b1c23ba..fe4ff9bade4d 100644 --- a/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java +++ b/core/trino-main/src/main/java/io/trino/server/HttpRequestSessionContextFactory.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.units.DataSize; import io.airlift.units.Duration; import io.trino.Session.ResourceEstimateBuilder; @@ -374,6 +375,7 @@ private static ResourceEstimates parseResourceEstimate(ProtocolHeaders protocolH return builder.build(); } + @FormatMethod private static void assertRequest(boolean expression, String format, Object... args) { if (!expression) { diff --git a/core/trino-main/src/main/java/io/trino/server/TrinoSystemRequirements.java b/core/trino-main/src/main/java/io/trino/server/TrinoSystemRequirements.java index a9c59538dd2d..c261a50e8400 100644 --- a/core/trino-main/src/main/java/io/trino/server/TrinoSystemRequirements.java +++ b/core/trino-main/src/main/java/io/trino/server/TrinoSystemRequirements.java @@ -15,6 +15,7 @@ import com.google.common.base.StandardSystemProperty; import com.google.common.collect.ImmutableSet; +import com.google.errorprone.annotations.FormatMethod; import com.sun.management.UnixOperatingSystemMXBean; import io.airlift.slice.Slice; import io.airlift.slice.Slices; @@ -165,12 +166,14 @@ public static void verifySystemTimeIsReasonable() } } + @FormatMethod private static void failRequirement(String format, Object... args) { System.err.println("ERROR: " + format(format, args)); System.exit(100); } + @FormatMethod private static void warnRequirement(String format, Object... args) { System.err.println("WARNING: " + format(format, args)); diff --git a/core/trino-main/src/main/java/io/trino/server/security/oauth2/NimbusOAuth2Client.java b/core/trino-main/src/main/java/io/trino/server/security/oauth2/NimbusOAuth2Client.java index 9860703bdf59..fcb985d60223 100644 --- a/core/trino-main/src/main/java/io/trino/server/security/oauth2/NimbusOAuth2Client.java +++ b/core/trino-main/src/main/java/io/trino/server/security/oauth2/NimbusOAuth2Client.java @@ -378,7 +378,7 @@ private Optional queryUserInfo(String accessToken) try { UserInfoResponse response = httpClient.execute(new UserInfoRequest(userinfoUrl.get(), new BearerAccessToken(accessToken)), UserInfoResponse::parse); if (!response.indicatesSuccess()) { - LOG.error("Received bad response from userinfo endpoint: " + response.toErrorResponse().getErrorObject()); + LOG.error("Received bad response from userinfo endpoint: %s", response.toErrorResponse().getErrorObject()); return Optional.empty(); } return Optional.of(response.toSuccessResponse().getUserInfo().toJWTClaimsSet()); 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 801af63fb29a..2ece19e0bf6e 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 @@ -14,6 +14,7 @@ package io.trino.sql.analyzer; import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.FormatMethod; import io.trino.Session; import io.trino.metadata.Metadata; import io.trino.spi.StandardErrorCode; @@ -438,8 +439,7 @@ protected Boolean visitFunctionCall(FunctionCall node, Void context) if (node.getFilter().isPresent()) { throw semanticException(FUNCTION_NOT_AGGREGATE, node, - "Filter is only valid for aggregation functions", - node); + "Filter is only valid for aggregation functions"); } if (node.getOrderBy().isPresent()) { throw semanticException(FUNCTION_NOT_AGGREGATE, node, "ORDER BY is only valid for aggregation functions"); @@ -787,12 +787,13 @@ private boolean hasOrderByReferencesToOutputColumns(Node node) return hasReferencesToScope(node, analysis, orderByScope.get()); } - private void verifyNoOrderByReferencesToOutputColumns(Node node, StandardErrorCode errorCode, String errorString) + @FormatMethod + private void verifyNoOrderByReferencesToOutputColumns(Node node, StandardErrorCode errorCode, String errorString, Object... errorStringArguments) { getReferencesToScope(node, analysis, orderByScope.get()) .findFirst() .ifPresent(expression -> { - throw semanticException(errorCode, expression, errorString); + throw semanticException(errorCode, expression, errorString, errorStringArguments); }); } } 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 3a21988710ea..94b4080bb438 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 @@ -22,6 +22,8 @@ import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Multimap; import com.google.common.collect.Streams; +import com.google.errorprone.annotations.FormatMethod; +import com.google.errorprone.annotations.FormatString; import io.trino.Session; import io.trino.execution.warnings.WarningCollector; import io.trino.metadata.OperatorNotFoundException; @@ -758,7 +760,7 @@ protected Type visitDereferenceExpression(DereferenceExpression node, StackableA for (RowType.Field rowField : rowType.getFields()) { if (fieldName.equalsIgnoreCase(rowField.getName().orElse(null))) { if (foundFieldName) { - throw semanticException(AMBIGUOUS_NAME, field, "Ambiguous row field reference: " + fieldName); + throw semanticException(AMBIGUOUS_NAME, field, "Ambiguous row field reference: %s", fieldName); } foundFieldName = true; rowFieldType = rowField.getType(); @@ -1243,7 +1245,7 @@ protected Type visitFunctionCall(FunctionCall node, StackableAstVisitorContext 254) { - throw semanticException(TOO_MANY_ARGUMENTS, node, "Too many arguments for array constructor", function.getSignature().getName()); + throw semanticException(TOO_MANY_ARGUMENTS, node, "Too many arguments for array constructor: %s", function.getSignature().getName()); } } else if (node.getArguments().size() > 127) { @@ -1491,7 +1493,7 @@ else if (frame.getType() == GROUPS) { } } else { - throw semanticException(NOT_SUPPORTED, frame, "Unsupported frame type: " + frame.getType()); + throw semanticException(NOT_SUPPORTED, frame, "Unsupported frame type: %s", frame.getType()); } } } @@ -2421,7 +2423,7 @@ protected Type visitLambdaExpression(LambdaExpression node, StackableAstVisitorC if (types.size() != lambdaArguments.size()) { throw semanticException(INVALID_PARAMETER_USAGE, node, - format("Expected a lambda that takes %s argument(s) but got %s", types.size(), lambdaArguments.size())); + "Expected a lambda that takes %s argument(s) but got %s", types.size(), lambdaArguments.size()); } ImmutableList.Builder fields = ImmutableList.builder(); @@ -2557,7 +2559,7 @@ public Type visitJsonValue(JsonValue node, StackableAstVisitorContext c !isDateTimeType(returnedType) || returnedType.equals(INTERVAL_DAY_TIME) || returnedType.equals(INTERVAL_YEAR_MONTH)) { - throw semanticException(TYPE_MISMATCH, node, "Invalid return type of function JSON_VALUE: " + node.getReturnedType().get()); + throw semanticException(TYPE_MISMATCH, node, "Invalid return type of function JSON_VALUE: %s", node.getReturnedType().get()); } JsonPathAnalysis pathAnalysis = jsonPathAnalyses.get(NodeRef.of(node)); @@ -2803,7 +2805,7 @@ private ResolvedFunction getInputFunction(Type type, JsonFormat format, Node nod if (isStringType(type)) { yield QualifiedName.of(VARBINARY_TO_JSON); } - throw semanticException(TYPE_MISMATCH, node, format("Cannot read input of type %s as JSON using formatting %s", type, format)); + throw semanticException(TYPE_MISMATCH, node, "Cannot read input of type %s as JSON using formatting %s", type, format); } case UTF8 -> QualifiedName.of(VARBINARY_UTF8_TO_JSON); case UTF16 -> QualifiedName.of(VARBINARY_UTF16_TO_JSON); @@ -2828,23 +2830,23 @@ private ResolvedFunction getOutputFunction(Type type, JsonFormat format, Node no if (isStringType(type)) { yield QualifiedName.of(JSON_TO_VARBINARY); } - throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format)); + throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format); } case UTF8 -> { if (!VARBINARY.equals(type)) { - throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format)); + throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format); } yield QualifiedName.of(JSON_TO_VARBINARY_UTF8); } case UTF16 -> { if (!VARBINARY.equals(type)) { - throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format)); + throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format); } yield QualifiedName.of(JSON_TO_VARBINARY_UTF16); } case UTF32 -> { if (!VARBINARY.equals(type)) { - throw semanticException(TYPE_MISMATCH, node, format("Cannot output JSON value as %s using formatting %s", type, format)); + throw semanticException(TYPE_MISMATCH, node, "Cannot output JSON value as %s using formatting %s", type, format); } yield QualifiedName.of(JSON_TO_VARBINARY_UTF32); } @@ -3137,7 +3139,8 @@ private void coerceType(StackableAstVisitorContext context, Expression coerceType(expression, actualType, expectedType, message); } - private Type coerceToSingleType(StackableAstVisitorContext context, Node node, String message, Expression first, Expression second) + @FormatMethod + private Type coerceToSingleType(StackableAstVisitorContext context, Node node, @FormatString String message, Expression first, Expression second) { Type firstType = UNKNOWN; if (first != null) { @@ -3163,7 +3166,9 @@ private Type coerceToSingleType(StackableAstVisitorContext context, Nod return superType; } - throw semanticException(TYPE_MISMATCH, node, message, firstType, secondType); + @SuppressWarnings("FormatStringAnnotation") // Error Prone wants the types of format arguments to be the same as where @FormatString is declared, but we need them to be different + TrinoException exception = semanticException(TYPE_MISMATCH, node, message, firstType, secondType); + throw exception; } private Type coerceToSingleType(StackableAstVisitorContext context, String description, List expressions) @@ -3184,12 +3189,12 @@ private Type coerceToSingleType(StackableAstVisitorContext context, Str for (Type type : types) { Optional newSuperType = typeCoercion.getCommonSuperType(superType, type); if (newSuperType.isEmpty()) { - throw semanticException(TYPE_MISMATCH, Iterables.get(typeExpressions.get(type), 0).getNode(), format( + throw semanticException(TYPE_MISMATCH, Iterables.get(typeExpressions.get(type), 0).getNode(), "%s must be the same type or coercible to a common type. Cannot find common type between %s and %s, all types (without duplicates): %s", description, superType, type, - typeExpressions.keySet())); + typeExpressions.keySet()); } superType = newSuperType.get(); } @@ -3200,12 +3205,12 @@ private Type coerceToSingleType(StackableAstVisitorContext context, Str if (!type.equals(superType)) { if (!typeCoercion.canCoerce(type, superType)) { - throw semanticException(TYPE_MISMATCH, Iterables.get(coercionCandidates, 0).getNode(), format( + throw semanticException(TYPE_MISMATCH, Iterables.get(coercionCandidates, 0).getNode(), "%s must be the same type or coercible to a common type. Cannot find common type between %s and %s, all types (without duplicates): %s", description, superType, type, - typeExpressions.keySet())); + typeExpressions.keySet()); } addOrReplaceExpressionsCoercion(coercionCandidates, type, superType); } @@ -3559,7 +3564,7 @@ public static ExpressionAnalyzer createWithoutSubqueries( session, TypeProvider.empty(), parameters, - node -> semanticException(errorCode, node, message), + node -> semanticException(errorCode, node, "%s", message), warningCollector, isDescribe); } diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/JsonPathAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/JsonPathAnalyzer.java index bbd78e82c233..f9ce682181b4 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/JsonPathAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/JsonPathAnalyzer.java @@ -83,7 +83,6 @@ import static io.trino.sql.analyzer.TypeSignatureProvider.fromTypes; import static io.trino.sql.jsonpath.tree.ArithmeticUnary.Sign.PLUS; import static io.trino.type.Json2016Type.JSON_2016; -import static java.lang.String.format; import static java.util.Objects.requireNonNull; public class JsonPathAnalyzer @@ -361,9 +360,9 @@ protected Type visitNamedVariable(NamedVariable node, Void context) .filter(name -> name.equalsIgnoreCase(node.getName())) .findFirst(); if (similarName.isPresent()) { - throw semanticException(INVALID_PATH, pathNode, format("no value passed for parameter %s. Try quoting \"%s\" in the PASSING clause to match case", node.getName(), node.getName())); + throw semanticException(INVALID_PATH, pathNode, "no value passed for parameter %s. Try quoting \"%s\" in the PASSING clause to match case", node.getName(), node.getName()); } - throw semanticException(INVALID_PATH, pathNode, "no value passed for parameter " + node.getName()); + throw semanticException(INVALID_PATH, pathNode, "no value passed for parameter %s", node.getName()); } if (parameterType.equals(JSON_2016)) { diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/SemanticExceptions.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/SemanticExceptions.java index 49f4f57fd9d9..4a11129a1cce 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/SemanticExceptions.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/SemanticExceptions.java @@ -13,6 +13,7 @@ */ package io.trino.sql.analyzer; +import com.google.errorprone.annotations.FormatMethod; import io.trino.spi.ErrorCodeSupplier; import io.trino.spi.TrinoException; import io.trino.sql.tree.Expression; @@ -38,11 +39,13 @@ public static TrinoException ambiguousAttributeException(Expression node, Qualif throw semanticException(AMBIGUOUS_NAME, node, "Column '%s' is ambiguous", name); } + @FormatMethod public static TrinoException semanticException(ErrorCodeSupplier code, Node node, String format, Object... args) { return semanticException(code, node, null, format, args); } + @FormatMethod public static TrinoException semanticException(ErrorCodeSupplier code, Node node, Throwable cause, String format, Object... args) { throw new TrinoException(code, extractLocation(node), format(format, args), cause); diff --git a/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java b/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java index 2cb355d0523a..46d95bc161bc 100644 --- a/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java +++ b/core/trino-main/src/main/java/io/trino/sql/gen/LambdaBytecodeGenerator.java @@ -308,7 +308,7 @@ private static Method getSingleApplyMethod(Class lambdaFunctionInterface) .filter(method -> method.getName().equals("apply")) .collect(toImmutableList()); - checkCondition(applyMethods.size() == 1, COMPILER_ERROR, "Expect to have exactly 1 method with name 'apply' in interface " + lambdaFunctionInterface.getName()); + checkCondition(applyMethods.size() == 1, COMPILER_ERROR, "Expect to have exactly 1 method with name 'apply' in interface %s", lambdaFunctionInterface.getName()); return applyMethods.get(0); } diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/ExtractSpatialJoins.java b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/ExtractSpatialJoins.java index a8c44e663fb0..b92f99d424e3 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/ExtractSpatialJoins.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/iterative/rule/ExtractSpatialJoins.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.errorprone.annotations.FormatMethod; import io.trino.Session; import io.trino.geospatial.KdbTree; import io.trino.geospatial.KdbTreeUtils; @@ -506,6 +507,7 @@ private static KdbTree loadKdbTree(String tableName, Session session, Metadata m return kdbTree.get(); } + @FormatMethod private static void checkSpatialPartitioningTable(boolean condition, String message, Object... arguments) { if (!condition) { diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/NodeRepresentation.java b/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/NodeRepresentation.java index 3e7a61619e63..0ecd9fcfaf61 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/NodeRepresentation.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/NodeRepresentation.java @@ -16,6 +16,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.FormatMethod; import io.trino.cost.LocalCostEstimate; import io.trino.cost.PlanCostEstimate; import io.trino.cost.PlanNodeStatsAndCostSummary; @@ -79,6 +80,7 @@ public NodeRepresentation( checkArgument(estimatedCost.size() == estimatedStats.size(), "size of cost and stats list does not match"); } + @FormatMethod public void appendDetails(String string, Object... args) { if (args.length == 0) { diff --git a/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/PlanPrinter.java b/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/PlanPrinter.java index 5bd1c626f1b6..b3a6001b433d 100644 --- a/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/PlanPrinter.java +++ b/core/trino-main/src/main/java/io/trino/sql/planner/planprinter/PlanPrinter.java @@ -975,7 +975,7 @@ public Void visitPatternRecognition(PatternRecognitionNode node, Context context context.tag()); if (node.getCommonBaseFrame().isPresent()) { - nodeOutput.appendDetails("base frame: " + formatFrame(node.getCommonBaseFrame().get())); + nodeOutput.appendDetails("base frame: %s", formatFrame(node.getCommonBaseFrame().get())); } for (Map.Entry entry : node.getWindowFunctions().entrySet()) { WindowNode.Function function = entry.getValue(); @@ -994,17 +994,17 @@ public Void visitPatternRecognition(PatternRecognitionNode node, Context context appendValuePointers(nodeOutput, entry.getValue().getExpressionAndValuePointers()); } if (node.getRowsPerMatch() != WINDOW) { - nodeOutput.appendDetails(formatRowsPerMatch(node.getRowsPerMatch())); + nodeOutput.appendDetails("%s", formatRowsPerMatch(node.getRowsPerMatch())); } - nodeOutput.appendDetails(formatSkipTo(node.getSkipToPosition(), node.getSkipToLabel())); - nodeOutput.appendDetails(format("pattern[%s] (%s)", node.getPattern(), node.isInitial() ? "INITIAL" : "SEEK")); - nodeOutput.appendDetails(format("subsets[%s]", node.getSubsets().entrySet().stream() + nodeOutput.appendDetails("%s", formatSkipTo(node.getSkipToPosition(), node.getSkipToLabel())); + nodeOutput.appendDetails("pattern[%s] (%s)", node.getPattern(), node.isInitial() ? "INITIAL" : "SEEK"); + nodeOutput.appendDetails("subsets[%s]", node.getSubsets().entrySet().stream() .map(subset -> subset.getKey().getName() + " := " + subset.getValue().stream() .map(IrLabel::getName) .collect(Collectors.joining(", ", "{", "}"))) - .collect(joining(", ")))); + .collect(joining(", "))); for (Map.Entry entry : node.getVariableDefinitions().entrySet()) { nodeOutput.appendDetails("%s := %s", entry.getKey().getName(), anonymizer.anonymize(unresolveFunctions(entry.getValue().getExpression()))); appendValuePointers(nodeOutput, entry.getValue()); @@ -1027,7 +1027,7 @@ private void appendValuePointers(NodeRepresentation nodeOutput, ExpressionAndVal String sourceSymbolName = expressionAndPointers.getClassifierSymbols().contains(symbol) ? "classifier" : anonymizer.anonymize(scalarPointer.getInputSymbol()); - nodeOutput.appendDetails(indentString(1) + anonymizer.anonymize(symbol) + " := " + sourceSymbolName + "[" + formatLogicalIndexPointer(scalarPointer.getLogicalIndexPointer()) + "]"); + nodeOutput.appendDetails("%s%s := %s[%s]", indentString(1), anonymizer.anonymize(symbol), sourceSymbolName, formatLogicalIndexPointer(scalarPointer.getLogicalIndexPointer())); } else if (pointer instanceof AggregationValuePointer aggregationPointer) { String processingMode = aggregationPointer.getSetDescriptor().isRunning() ? "RUNNING " : "FINAL "; @@ -1036,7 +1036,7 @@ else if (pointer instanceof AggregationValuePointer aggregationPointer) { String labels = aggregationPointer.getSetDescriptor().getLabels().stream() .map(IrLabel::getName) .collect(joining(", ", "{", "}")); - nodeOutput.appendDetails(indentString(1) + anonymizer.anonymize(symbol) + " := " + processingMode + name + "(" + arguments + ")" + labels); + nodeOutput.appendDetails("%s%s := %s%s(%s)%s", indentString(1), anonymizer.anonymize(symbol), processingMode, name, arguments, labels); } else { throw new UnsupportedOperationException("unexpected ValuePointer type: " + pointer.getClass().getSimpleName()); @@ -1165,15 +1165,19 @@ public Void visitTableScan(TableScanNode node, Context context) printTableScanInfo(nodeOutput, node, tableInfo); PlanNodeStats nodeStats = stats.map(s -> s.get(node.getId())).orElse(null); if (nodeStats != null) { - String inputDetail = "Input: %s (%s)"; if (nodeStats.getPlanNodePhysicalInputDataSize().toBytes() > 0) { - inputDetail += ", Physical Input: %s"; + nodeOutput.appendDetails( + "Input: %s (%s), Physical Input: %s", + formatPositions(nodeStats.getPlanNodeInputPositions()), + nodeStats.getPlanNodeInputDataSize().toString(), + nodeStats.getPlanNodePhysicalInputDataSize().toString()); + } + else { + nodeOutput.appendDetails( + "Input: %s (%s)", + formatPositions(nodeStats.getPlanNodeInputPositions()), + nodeStats.getPlanNodeInputDataSize().toString()); } - nodeOutput.appendDetails( - inputDetail, - formatPositions(nodeStats.getPlanNodeInputPositions()), - nodeStats.getPlanNodeInputDataSize().toString(), - nodeStats.getPlanNodePhysicalInputDataSize().toString()); } return null; } @@ -1200,7 +1204,7 @@ public Void visitValues(ValuesNode node, Context context) }) .collect(toImmutableList()); for (String row : rows) { - nodeOutput.appendDetails(row); + nodeOutput.appendDetails("%s", row); } return null; } @@ -1293,16 +1297,21 @@ private Void visitScanFilterAndProjectInfo( if (nodeStats != null) { // Add to 'details' rather than 'statistics', since these stats are node-specific double filtered = 100.0d * (nodeStats.getPlanNodeInputPositions() - nodeStats.getPlanNodeOutputPositions()) / nodeStats.getPlanNodeInputPositions(); - String inputDetail = "Input: %s (%s), Filtered: %s%%"; if (nodeStats.getPlanNodePhysicalInputDataSize().toBytes() > 0) { - inputDetail += ", Physical Input: %s"; + nodeOutput.appendDetails( + "Input: %s (%s), Filtered: %s%%, Physical Input: %s", + formatPositions(nodeStats.getPlanNodeInputPositions()), + nodeStats.getPlanNodeInputDataSize().toString(), + formatDouble(filtered), + nodeStats.getPlanNodePhysicalInputDataSize().toString()); + } + else { + nodeOutput.appendDetails( + "Input: %s (%s), Filtered: %s%%", + formatPositions(nodeStats.getPlanNodeInputPositions()), + nodeStats.getPlanNodeInputDataSize().toString(), + formatDouble(filtered)); } - nodeOutput.appendDetails( - inputDetail, - formatPositions(nodeStats.getPlanNodeInputPositions()), - nodeStats.getPlanNodeInputDataSize().toString(), - formatDouble(filtered), - nodeStats.getPlanNodePhysicalInputDataSize().toString()); } List collectedDomainStats = dynamicFilters.stream() .map(DynamicFilters.Descriptor::getId) @@ -1555,7 +1564,7 @@ private void printStatisticAggregations(NodeRepresentation nodeOutput, Statistic { nodeOutput.appendDetails("Collected statistics:"); printStatisticAggregationsInfo(nodeOutput, descriptor.getTableStatistics(), descriptor.getColumnStatistics(), aggregations.getAggregations()); - nodeOutput.appendDetails(indentString(1) + "grouped by => [%s]", getStatisticGroupingSetsInfo(descriptor.getGrouping())); + nodeOutput.appendDetails("%sgrouped by => [%s]", indentString(1), getStatisticGroupingSetsInfo(descriptor.getGrouping())); } private String getStatisticGroupingSetsInfo(Map columnMappings) @@ -1573,7 +1582,8 @@ private void printStatisticAggregationsInfo( { nodeOutput.appendDetails("aggregations =>"); for (Map.Entry tableStatistic : tableStatistics.entrySet()) { - nodeOutput.appendDetails(indentString(1) + "%s => [%s := %s]", + nodeOutput.appendDetails("%s%s => [%s := %s]", + indentString(1), anonymizer.anonymize(tableStatistic.getValue()), tableStatistic.getKey(), formatAggregation(anonymizer, aggregations.get(tableStatistic.getValue()))); @@ -1594,7 +1604,8 @@ private void printStatisticAggregationsInfo( } } nodeOutput.appendDetails( - indentString(1) + "%s[%s] => [%s := %s]", + "%s%s[%s] => [%s := %s]", + indentString(1), aggregationName, anonymizer.anonymizeColumn(columnStatistic.getKey().getColumnName()), anonymizer.anonymize(columnStatistic.getValue()), @@ -1774,10 +1785,10 @@ public Void visitTableFunction(TableFunctionNode node, Context context) .collect(toImmutableMap(TableArgumentProperties::getArgumentName, identity())); node.getArguments().entrySet() - .forEach(entry -> nodeOutput.appendDetails(formatArgument(entry.getKey(), entry.getValue(), tableArguments))); + .forEach(entry -> nodeOutput.appendDetails("%s", formatArgument(entry.getKey(), entry.getValue(), tableArguments))); if (!node.getCopartitioningLists().isEmpty()) { - nodeOutput.appendDetails(node.getCopartitioningLists().stream() + nodeOutput.appendDetails("%s", node.getCopartitioningLists().stream() .map(list -> list.stream().collect(Collectors.joining(", ", "(", ")"))) .collect(joining(", ", "Co-partition: [", "]"))); } diff --git a/core/trino-main/src/main/java/io/trino/util/Failures.java b/core/trino-main/src/main/java/io/trino/util/Failures.java index 43a86fba0c5c..62756e6c3c70 100644 --- a/core/trino-main/src/main/java/io/trino/util/Failures.java +++ b/core/trino-main/src/main/java/io/trino/util/Failures.java @@ -14,6 +14,7 @@ package io.trino.util; import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.FormatMethod; import io.trino.client.ErrorLocation; import io.trino.execution.ExecutionFailureInfo; import io.trino.execution.Failure; @@ -58,6 +59,7 @@ public static ExecutionFailureInfo toFailure(Throwable failure) return toFailure(failure, newIdentityHashSet()); } + @FormatMethod public static void checkCondition(boolean condition, ErrorCodeSupplier errorCode, String formatString, Object... args) { if (!condition) { diff --git a/core/trino-main/src/test/java/io/trino/cost/EstimateAssertion.java b/core/trino-main/src/test/java/io/trino/cost/EstimateAssertion.java index 8d231a6559cd..8636a26d2480 100644 --- a/core/trino-main/src/test/java/io/trino/cost/EstimateAssertion.java +++ b/core/trino-main/src/test/java/io/trino/cost/EstimateAssertion.java @@ -13,6 +13,7 @@ */ package io.trino.cost; +import com.google.errorprone.annotations.FormatMethod; import io.trino.util.MoreMath; import static java.lang.Double.isNaN; @@ -24,6 +25,7 @@ private EstimateAssertion() {} private static final double TOLERANCE = 0.0000001; + @FormatMethod public static void assertEstimateEquals(double actual, double expected, String messageFormat, Object... messageObjects) { if (isNaN(actual) && isNaN(expected)) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/QueryId.java b/core/trino-spi/src/main/java/io/trino/spi/QueryId.java index 956aead6da45..9af82680a02a 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/QueryId.java +++ b/core/trino-spi/src/main/java/io/trino/spi/QueryId.java @@ -113,6 +113,7 @@ public static List parseDottedId(String id, int expectedParts, String na return ids; } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean condition, String message, Object... messageArgs) { if (!condition) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/InMemoryRecordSet.java b/core/trino-spi/src/main/java/io/trino/spi/connector/InMemoryRecordSet.java index 5463f47c0fc2..f8d1afe3b5c8 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/InMemoryRecordSet.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/InMemoryRecordSet.java @@ -271,6 +271,7 @@ public InMemoryRecordSet build() } } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean test, String message, Object... args) { if (!test) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/resourcegroups/ResourceGroupId.java b/core/trino-spi/src/main/java/io/trino/spi/resourcegroups/ResourceGroupId.java index b589686aed03..3248546db47a 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/resourcegroups/ResourceGroupId.java +++ b/core/trino-spi/src/main/java/io/trino/spi/resourcegroups/ResourceGroupId.java @@ -90,6 +90,7 @@ public boolean isAncestorOf(ResourceGroupId descendant) return descendantSegments.subList(0, segments.size()).equals(segments); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean argument, String format, Object... args) { if (!argument) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/DecimalType.java b/core/trino-spi/src/main/java/io/trino/spi/type/DecimalType.java index efaaba6a2f51..615659ff1b42 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/DecimalType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/DecimalType.java @@ -99,6 +99,7 @@ private static List buildTypeParameters(int precision, i return List.of(numericParameter(precision), numericParameter(scale)); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency static void checkArgument(boolean condition, String format, Object... args) { if (!condition) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/QuantileDigestParametricType.java b/core/trino-spi/src/main/java/io/trino/spi/type/QuantileDigestParametricType.java index 90b1d6a8f92e..1d2cd8d1aea0 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/QuantileDigestParametricType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/QuantileDigestParametricType.java @@ -41,6 +41,7 @@ public Type createType(TypeManager typeManager, List parameters) return new QuantileDigestType(parameters.get(0).getType()); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean argument, String format, Object... args) { if (!argument) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/TimeZoneKey.java b/core/trino-spi/src/main/java/io/trino/spi/type/TimeZoneKey.java index 647e4a994669..47bd708850f1 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/TimeZoneKey.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/TimeZoneKey.java @@ -348,6 +348,7 @@ private static String zoneIdForOffset(long offsetMinutes) return formatZoneOffset(offsetMinutes >= 0, toIntExact(abs(offsetMinutes / 60)), (int) abs(offsetMinutes % 60)); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean check, String message, Object... args) { if (!check) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/TypeOperatorDeclaration.java b/core/trino-spi/src/main/java/io/trino/spi/type/TypeOperatorDeclaration.java index 7204ac95f380..93ea5a1d0a79 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/TypeOperatorDeclaration.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/TypeOperatorDeclaration.java @@ -516,6 +516,7 @@ else if (parameterTypes.size() > 1 && isAnnotationPresent(parameterAnnotations.g throw new IllegalArgumentException(format("Unexpected parameters for %s operator: %s", operatorType, method)); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean test, String message, Object... arguments) { if (!test) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/TypeSignature.java b/core/trino-spi/src/main/java/io/trino/spi/type/TypeSignature.java index bbc69cf6b8ec..f46f3fd196e8 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/TypeSignature.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/TypeSignature.java @@ -134,6 +134,7 @@ private String formatValue(boolean json) return typeName.toString(); } + @SuppressWarnings("AnnotateFormatMethod") // would require adding error_prone_annotations dependency private static void checkArgument(boolean argument, String format, Object... args) { if (!argument) { diff --git a/lib/trino-hive-formats/pom.xml b/lib/trino-hive-formats/pom.xml index 03d915cb137e..ddcf9aae48ef 100644 --- a/lib/trino-hive-formats/pom.xml +++ b/lib/trino-hive-formats/pom.xml @@ -60,6 +60,11 @@ true + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileCorruptionException.java b/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileCorruptionException.java index 7720660b985c..87bf3da0908e 100644 --- a/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileCorruptionException.java +++ b/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileCorruptionException.java @@ -13,6 +13,8 @@ */ package io.trino.hive.formats.rcfile; +import com.google.errorprone.annotations.FormatMethod; + import java.io.IOException; import static java.lang.String.format; @@ -25,11 +27,13 @@ public RcFileCorruptionException(String message) super(message); } + @FormatMethod public RcFileCorruptionException(String messageFormat, Object... args) { super(format(messageFormat, args)); } + @FormatMethod public RcFileCorruptionException(Throwable cause, String messageFormat, Object... args) { super(format(messageFormat, args), cause); diff --git a/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileReader.java b/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileReader.java index 979823dd1f9d..f5c653aa4b96 100644 --- a/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileReader.java +++ b/lib/trino-hive-formats/src/main/java/io/trino/hive/formats/rcfile/RcFileReader.java @@ -14,6 +14,7 @@ package io.trino.hive.formats.rcfile; import com.google.common.collect.ImmutableMap; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.slice.BasicSliceInput; import io.airlift.slice.Slice; import io.airlift.slice.Slices; @@ -458,6 +459,7 @@ private Slice readLengthPrefixedString(DataSeekableInputStream in) return in.readSlice(length); } + @FormatMethod private void verify(boolean expression, String messageFormat, Object... args) throws RcFileCorruptionException { @@ -466,17 +468,21 @@ private void verify(boolean expression, String messageFormat, Object... args) } } + @FormatMethod private RcFileCorruptionException corrupt(String messageFormat, Object... args) { closeQuietly(); return new RcFileCorruptionException(messageFormat, args); } + @FormatMethod private void validateWrite(Predicate test, String messageFormat, Object... args) throws RcFileCorruptionException { if (writeValidation.isPresent() && !test.test(writeValidation.get())) { - throw corrupt("Write validation failed: " + messageFormat, args); + @SuppressWarnings("FormatStringAnnotation") + RcFileCorruptionException exception = corrupt("Write validation failed: " + messageFormat, args); + throw exception; } } diff --git a/lib/trino-orc/pom.xml b/lib/trino-orc/pom.xml index e42d13b6952b..5987cd4a485b 100644 --- a/lib/trino-orc/pom.xml +++ b/lib/trino-orc/pom.xml @@ -67,6 +67,11 @@ units + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/lib/trino-orc/src/main/java/io/trino/orc/checkpoint/InvalidCheckpointException.java b/lib/trino-orc/src/main/java/io/trino/orc/checkpoint/InvalidCheckpointException.java index 0d3a0b51414f..116c845430cb 100644 --- a/lib/trino-orc/src/main/java/io/trino/orc/checkpoint/InvalidCheckpointException.java +++ b/lib/trino-orc/src/main/java/io/trino/orc/checkpoint/InvalidCheckpointException.java @@ -13,11 +13,14 @@ */ package io.trino.orc.checkpoint; +import com.google.errorprone.annotations.FormatMethod; + import static java.lang.String.format; public class InvalidCheckpointException extends RuntimeException { + @FormatMethod public InvalidCheckpointException(String message, Object... arguments) { super(format(message, arguments)); diff --git a/lib/trino-parquet/pom.xml b/lib/trino-parquet/pom.xml index 743568dce35a..c214f97012b3 100644 --- a/lib/trino-parquet/pom.xml +++ b/lib/trino-parquet/pom.xml @@ -58,6 +58,11 @@ true + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetCorruptionException.java b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetCorruptionException.java index 719190d336ea..b24533b46600 100644 --- a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetCorruptionException.java +++ b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetCorruptionException.java @@ -13,6 +13,8 @@ */ package io.trino.parquet; +import com.google.errorprone.annotations.FormatMethod; + import java.io.IOException; import static java.lang.String.format; @@ -25,22 +27,26 @@ public ParquetCorruptionException(String message) super(message); } + @FormatMethod public ParquetCorruptionException(String messageFormat, Object... args) { super(format(messageFormat, args)); } + @FormatMethod public ParquetCorruptionException(Throwable cause, String messageFormat, Object... args) { super(format(messageFormat, args), cause); } + @FormatMethod public ParquetCorruptionException(ParquetDataSourceId dataSourceId, String messageFormat, Object... args) { super(formatMessage(dataSourceId, messageFormat, args)); } - private static String formatMessage(ParquetDataSourceId dataSourceId, String messageFormat, Object[] args) + @FormatMethod + private static String formatMessage(ParquetDataSourceId dataSourceId, String messageFormat, Object... args) { return "Malformed Parquet file. " + format(messageFormat, args) + " [" + dataSourceId + "]"; } diff --git a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetValidationUtils.java b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetValidationUtils.java index c194405c0029..9be1c4477947 100644 --- a/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetValidationUtils.java +++ b/lib/trino-parquet/src/main/java/io/trino/parquet/ParquetValidationUtils.java @@ -13,12 +13,15 @@ */ package io.trino.parquet; +import com.google.errorprone.annotations.FormatMethod; + import static java.lang.String.format; public final class ParquetValidationUtils { private ParquetValidationUtils() {} + @FormatMethod public static void validateParquet(boolean condition, String formatString, Object... args) throws ParquetCorruptionException { @@ -27,6 +30,7 @@ public static void validateParquet(boolean condition, String formatString, Objec } } + @FormatMethod public static void validateParquet(boolean condition, ParquetDataSourceId dataSourceId, String formatString, Object... args) throws ParquetCorruptionException { diff --git a/lib/trino-parquet/src/main/java/io/trino/parquet/predicate/TupleDomainParquetPredicate.java b/lib/trino-parquet/src/main/java/io/trino/parquet/predicate/TupleDomainParquetPredicate.java index 49b8e3ca7346..527bc883f327 100644 --- a/lib/trino-parquet/src/main/java/io/trino/parquet/predicate/TupleDomainParquetPredicate.java +++ b/lib/trino-parquet/src/main/java/io/trino/parquet/predicate/TupleDomainParquetPredicate.java @@ -593,12 +593,12 @@ private static Domain getDomain(Type type, DictionaryDescriptor dictionaryDescri private static ParquetCorruptionException corruptionException(String column, ParquetDataSourceId id, Statistics statistics, Exception cause) { - return new ParquetCorruptionException(cause, format("Corrupted statistics for column \"%s\" in Parquet file \"%s\": [%s]", column, id, statistics)); + return new ParquetCorruptionException(cause, "Corrupted statistics for column \"%s\" in Parquet file \"%s\": [%s]", column, id, statistics); } private static ParquetCorruptionException corruptionException(String column, ParquetDataSourceId id, ColumnIndex columnIndex, Exception cause) { - return new ParquetCorruptionException(cause, format("Corrupted statistics for column \"%s\" in Parquet file \"%s\". Corrupted column index: [%s]", column, id, columnIndex)); + return new ParquetCorruptionException(cause, "Corrupted statistics for column \"%s\" in Parquet file \"%s\". Corrupted column index: [%s]", column, id, columnIndex); } private static boolean isCorruptedColumnIndex( diff --git a/lib/trino-parquet/src/main/java/io/trino/parquet/reader/ParquetReader.java b/lib/trino-parquet/src/main/java/io/trino/parquet/reader/ParquetReader.java index a05c1981af7d..a31df0840ff6 100644 --- a/lib/trino-parquet/src/main/java/io/trino/parquet/reader/ParquetReader.java +++ b/lib/trino-parquet/src/main/java/io/trino/parquet/reader/ParquetReader.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ListMultimap; +import com.google.errorprone.annotations.FormatMethod; import io.trino.memory.context.AggregatedMemoryContext; import io.trino.parquet.ChunkKey; import io.trino.parquet.DiskRange; @@ -590,11 +591,14 @@ private void validateBlockMetadata(List blockMetaData) } } + @FormatMethod private void validateWrite(java.util.function.Predicate test, String messageFormat, Object... args) throws ParquetCorruptionException { if (writeValidation.isPresent() && !test.test(writeValidation.get())) { - throw new ParquetCorruptionException(dataSource.getId(), "Write validation failed: " + messageFormat, args); + @SuppressWarnings("FormatStringAnnotation") + ParquetCorruptionException exception = new ParquetCorruptionException(dataSource.getId(), "Write validation failed: " + messageFormat, args); + throw exception; } } } diff --git a/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraSession.java b/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraSession.java index 1517f414abca..faddea000e26 100644 --- a/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraSession.java +++ b/plugin/trino-cassandra/src/main/java/io/trino/plugin/cassandra/CassandraSession.java @@ -571,7 +571,7 @@ private T executeWithSession(SessionCallable sessionCallable) throw e; } long delay = Math.min(schedule.nextDelay().toMillis(), timeLeft); - log.warn(e.getMessage()); + log.warn("%s", e.getMessage()); log.warn("Reconnecting in %dms", delay); try { Thread.sleep(delay); diff --git a/plugin/trino-geospatial/pom.xml b/plugin/trino-geospatial/pom.xml index 779d8b880bb7..ee7df817eeca 100644 --- a/plugin/trino-geospatial/pom.xml +++ b/plugin/trino-geospatial/pom.xml @@ -32,6 +32,11 @@ esri-geometry-api + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/plugin/trino-geospatial/src/main/java/io/trino/plugin/geospatial/BingTileFunctions.java b/plugin/trino-geospatial/src/main/java/io/trino/plugin/geospatial/BingTileFunctions.java index 6dbc00ebe57a..731c6fb8cbda 100644 --- a/plugin/trino-geospatial/src/main/java/io/trino/plugin/geospatial/BingTileFunctions.java +++ b/plugin/trino-geospatial/src/main/java/io/trino/plugin/geospatial/BingTileFunctions.java @@ -17,6 +17,7 @@ import com.esri.core.geometry.Point; import com.esri.core.geometry.ogc.OGCGeometry; import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.slice.Slice; import io.trino.spi.PageBuilder; import io.trino.spi.TrinoException; @@ -661,12 +662,12 @@ private static void checkQuadKey(@SqlType(StandardTypes.VARCHAR) Slice quadkey) private static void checkLatitude(double latitude, String errorMessage) { - checkCondition(latitude >= MIN_LATITUDE && latitude <= MAX_LATITUDE, errorMessage); + checkCondition(latitude >= MIN_LATITUDE && latitude <= MAX_LATITUDE, "%s", errorMessage); } private static void checkLongitude(double longitude, String errorMessage) { - checkCondition(longitude >= MIN_LONGITUDE && longitude <= MAX_LONGITUDE, errorMessage); + checkCondition(longitude >= MIN_LONGITUDE && longitude <= MAX_LONGITUDE, "%s", errorMessage); } private static boolean withinDistance(GreatCircleDistanceToPoint distanceFunction, double maxDistance, Point point) @@ -706,6 +707,7 @@ public double distance(double latitude2, double longitude2) } } + @FormatMethod private static void checkCondition(boolean condition, String formatString, Object... args) { if (!condition) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java index d07446ff6920..9855ccb5a7da 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java @@ -1134,7 +1134,7 @@ private AWSCredentialsProvider createAwsCredentialsProvider(URI uri, Configurati region = regionProviderChain.getRegion(); } catch (SdkClientException ex) { - log.warn("Falling back to default AWS region " + US_EAST_1); + log.warn("Falling back to default AWS region %s", US_EAST_1); region = US_EAST_1.getName(); } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/statistics/MetastoreHiveStatisticsProvider.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/statistics/MetastoreHiveStatisticsProvider.java index fea2526be56d..d13da798597f 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/statistics/MetastoreHiveStatisticsProvider.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/statistics/MetastoreHiveStatisticsProvider.java @@ -21,6 +21,8 @@ import com.google.common.primitives.Ints; import com.google.common.primitives.Shorts; import com.google.common.primitives.SignedBytes; +import com.google.errorprone.annotations.FormatMethod; +import com.google.errorprone.annotations.FormatString; import io.airlift.log.Logger; import io.airlift.slice.Slice; import io.trino.plugin.hive.HiveBasicStatistics; @@ -364,7 +366,8 @@ private static void validateColumnStatistics(SchemaTableName table, String parti }); } - private static void checkStatistics(boolean expression, SchemaTableName table, String partition, String column, String message, Object... args) + @FormatMethod + private static void checkStatistics(boolean expression, SchemaTableName table, String partition, String column, @FormatString String message, Object... args) { if (!expression) { throw new TrinoException( @@ -373,7 +376,8 @@ private static void checkStatistics(boolean expression, SchemaTableName table, S } } - private static void checkStatistics(boolean expression, SchemaTableName table, String partition, String message, Object... args) + @FormatMethod + private static void checkStatistics(boolean expression, SchemaTableName table, String partition, @FormatString String message, Object... args) { if (!expression) { throw new TrinoException( diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java index aae17e135c8f..241110131d3c 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveUtil.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.compress.lzo.LzoCodec; import io.airlift.compress.lzo.LzopCodec; import io.airlift.slice.Slice; @@ -915,6 +916,7 @@ public static List getPartitionKeyColumnHandles(Table table, T return columns.build(); } + @FormatMethod public static void checkCondition(boolean condition, ErrorCodeSupplier errorCode, String formatString, Object... args) { if (!condition) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java index 3dadd09d8f29..fc8617db63c5 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java @@ -1310,7 +1310,7 @@ public void executeRemoveOrphanFiles(ConnectorSession session, IcebergTableExecu IcebergSessionProperties.REMOVE_ORPHAN_FILES_MIN_RETENTION); if (table.currentSnapshot() == null) { - log.debug("Skipping remove_orphan_files procedure for empty table " + table); + log.debug("Skipping remove_orphan_files procedure for empty table %s", table); return; } diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java index 3d78d6916061..9cbf6e6e098d 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java @@ -311,7 +311,7 @@ public void testView() catalog.dropNamespace(SESSION, namespace); } catch (Exception e) { - LOG.warn("Failed to clean up namespace: " + namespace); + LOG.warn("Failed to clean up namespace: %s", namespace); } } } diff --git a/pom.xml b/pom.xml index 1ae8edee5ea0..ce85c012260a 100644 --- a/pom.xml +++ b/pom.xml @@ -2174,6 +2174,7 @@ -XDcompilePolicy=simple -Xplugin:ErrorProne \ + -Xep:AnnotateFormatMethod:ERROR \ -Xep:BadInstanceof:ERROR \ -Xep:BoxedPrimitiveConstructor:ERROR \ -Xep:ClassCanBeStatic:ERROR \ diff --git a/service/trino-verifier/pom.xml b/service/trino-verifier/pom.xml index 6bb43e85d195..76425425dfa3 100644 --- a/service/trino-verifier/pom.xml +++ b/service/trino-verifier/pom.xml @@ -79,6 +79,11 @@ true + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/service/trino-verifier/src/main/java/io/trino/verifier/QueryRewriter.java b/service/trino-verifier/src/main/java/io/trino/verifier/QueryRewriter.java index 008193fc9916..5463d335053f 100644 --- a/service/trino-verifier/src/main/java/io/trino/verifier/QueryRewriter.java +++ b/service/trino-verifier/src/main/java/io/trino/verifier/QueryRewriter.java @@ -18,6 +18,7 @@ import com.google.common.util.concurrent.SimpleTimeLimiter; import com.google.common.util.concurrent.TimeLimiter; import com.google.common.util.concurrent.UncheckedTimeoutException; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.units.Duration; import io.trino.sql.parser.ParsingOptions; import io.trino.sql.parser.SqlParser; @@ -115,7 +116,7 @@ public Query shadowQuery(Query query) } } - throw new QueryRewriteException("Unsupported query type: " + statement.getClass()); + throw new QueryRewriteException("Unsupported query type: %s", statement.getClass()); } private Query rewriteCreateTableAsSelect(Connection connection, Query query, CreateTableAsSelect statement) @@ -241,7 +242,7 @@ private String checksumSql(List columns, QualifiedName table) throws QueryRewriteException { if (columns.isEmpty()) { - throw new QueryRewriteException("Table " + table + " has no columns"); + throw new QueryRewriteException("Table %s has no columns", table); } ImmutableList.Builder selectItems = ImmutableList.builder(); for (Column column : columns) { @@ -271,6 +272,7 @@ private static String escapeLikeExpression(Connection connection, String value) public static class QueryRewriteException extends Exception { + @FormatMethod public QueryRewriteException(String messageFormat, Object... args) { super(format(messageFormat, args)); diff --git a/testing/trino-tests/pom.xml b/testing/trino-tests/pom.xml index b4a83b9b427e..2e843228d84f 100644 --- a/testing/trino-tests/pom.xml +++ b/testing/trino-tests/pom.xml @@ -82,6 +82,12 @@ runtime + + com.google.errorprone + error_prone_annotations + runtime + + com.google.guava guava diff --git a/testing/trino-tests/src/test/java/io/trino/sql/planner/BaseCostBasedPlanTest.java b/testing/trino-tests/src/test/java/io/trino/sql/planner/BaseCostBasedPlanTest.java index 31192d895f9a..1169eb4107d9 100644 --- a/testing/trino-tests/src/test/java/io/trino/sql/planner/BaseCostBasedPlanTest.java +++ b/testing/trino-tests/src/test/java/io/trino/sql/planner/BaseCostBasedPlanTest.java @@ -17,6 +17,7 @@ import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableMap; import com.google.common.io.Resources; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.log.Logger; import io.trino.Session; import io.trino.execution.warnings.WarningCollector; @@ -337,6 +338,7 @@ public Void visitValues(ValuesNode node, Integer indent) return null; } + @FormatMethod private void output(int indent, String message, Object... args) { String formattedMessage = format(message, args);