Skip to content

Call semanticException method as a format method in several places #15051

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
kokosing merged 5 commits into from
Nov 17, 2022
Merged

Call semanticException method as a format method in several places #15051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d1e070c
Fix a typo in comment
ksobolew Nov 8, 2022
c4eb2e5
Call semanticException method as a format method in CommentTask
ksobolew Nov 8, 2022
8015105
Call semanticException method as a format method in CreateTableTask
ksobolew Nov 8, 2022
556fc72
Call semanticException method as a format method in SetPropertiesTask
ksobolew Nov 8, 2022
73db986
Call semanticException method as a format method in StatementAnalyzer
ksobolew Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions core/trino-main/src/main/java/io/trino/execution/CommentTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand All @@ -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<String, ColumnHandle> 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));
Expand Down
14 changes: 10 additions & 4 deletions core/trino-main/src/main/java/io/trino/execution/CreateTableTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 7 additions & 5 deletions core/trino-main/src/main/java/io/trino/execution/SetPropertiesTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -132,14 +131,17 @@ private void setMaterializedViewProperties(
Map<String, Optional<Object>> 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);
Expand Down
53 changes: 29 additions & 24 deletions core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Column> columns = Streams.zip(
Expand Down Expand Up @@ -1136,7 +1139,7 @@ protected Scope visitTableExecute(TableExecute node, Optional<Scope> 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));

Expand Down Expand Up @@ -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 <table reference>, p. 409,
Expand Down Expand Up @@ -1660,11 +1663,11 @@ private ArgumentsAnalysis analyzeArguments(List<ArgumentSpecification> 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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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())) {
Expand Down Expand Up @@ -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<ResolvedField> field = inputScope.tryResolveField(expression, qualifiedName);
if (field.isEmpty() || !field.get().isLocal()) {
Expand Down Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -2949,12 +2952,13 @@ protected Scope visitSetOperation(SetOperation node, Optional<Scope> 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" : "");
}
}
}
Expand Down Expand Up @@ -3619,7 +3623,7 @@ private ResolvedWindow resolveWindowSpecification(QuerySpecification querySpecif
CanonicalizationAware<Identifier> 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(
Expand All @@ -3638,7 +3642,7 @@ private ResolvedWindow resolveWindowSpecification(QuerySpecification querySpecif
CanonicalizationAware<Identifier> 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
Expand Down Expand Up @@ -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");
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion testing/trino-testing-services/src/main/java/io/trino/testng/services/Listeners.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<? extends ITestNGListener> listenerClass, String format, Object... args)
{
Expand Down