Skip to content

Throw TableNotFoundException when SQLServer error is 208 #16487

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
ebyhr merged 2 commits into from
Mar 10, 2023
Merged

Throw TableNotFoundException when SQLServer error is 208 #16487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d43230b
Uppercase 'from' in SQLServer getCaseSensitivityForColumns
ebyhr Mar 10, 2023
979aa96
Throw TableNotFoundException when SQLServer error is 208
ebyhr Mar 10, 2023
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
10 changes: 9 additions & 1 deletion plugin/trino-sqlserver/src/main/java/io/trino/plugin/sqlserver/SqlServerClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import io.trino.spi.connector.JoinCondition;
import io.trino.spi.connector.JoinStatistics;
import io.trino.spi.connector.JoinType;
import io.trino.spi.connector.TableNotFoundException;
import io.trino.spi.expression.ConnectorExpression;
import io.trino.spi.predicate.Domain;
import io.trino.spi.predicate.ValueSet;
Expand Down Expand Up @@ -438,7 +439,7 @@ protected Map<String, CaseSensitivity> getCaseSensitivityForColumns(ConnectorSes
if (tableHandle.isSynthetic()) {
return ImmutableMap.of();
}
PreparedQuery preparedQuery = new PreparedQuery(format("SELECT * from %s", quoted(tableHandle.asPlainTable().getRemoteTableName())), ImmutableList.of());
PreparedQuery preparedQuery = new PreparedQuery(format("SELECT * FROM %s", quoted(tableHandle.asPlainTable().getRemoteTableName())), ImmutableList.of());

try (PreparedStatement preparedStatement = queryBuilder.prepareStatement(this, session, connection, preparedQuery)) {
ResultSetMetaData metadata = preparedStatement.getMetaData();
Expand All @@ -450,6 +451,13 @@ protected Map<String, CaseSensitivity> getCaseSensitivityForColumns(ConnectorSes
return columns.buildOrThrow();
}
catch (SQLException e) {
if (e instanceof SQLServerException sqlServerException && sqlServerException.getSQLServerError().getErrorNumber() == 208) {
// The 208 indicates that the object doesn't exist or lack of permission.
// Throw TableNotFoundException because users shouldn't see such tables if they don't have the permission.
// TableNotFoundException will be suppressed when listing information_schema.
// https://learn.microsoft.com/sql/relational-databases/errors-events/mssqlserver-208-database-engine-error
throw new TableNotFoundException(tableHandle.asPlainTable().getSchemaTableName());
}
throw new TrinoException(JDBC_ERROR, "Failed to get case sensitivity for columns. " + firstNonNull(e.getMessage(), e), e);
}
}
Expand Down
8 changes: 0 additions & 8 deletions ...n/trino-sqlserver/src/test/java/io/trino/plugin/sqlserver/BaseSqlServerConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,6 @@ public void testReadMetadataWithRelationsConcurrentModifications()
}
catch (Exception expected) {
// The test failure is not guaranteed
// TODO (https://github.com/trinodb/trino/issues/10846): shouldn't fail
if (expected.getMessage().contains("case sensitivity")) {
// this could fail on step before - during read case sensitivity information
assertThat(expected)
// before get columns from database metadata, for SqlServer we first try to get case sensitivity
.hasMessageMatching(".*Failed to get case sensitivity for columns. Invalid object name.*");
throw new SkipException("to be fixed");
}
assertThat(expected)
.hasMessageMatching("(?s).*(" +
"No task completed before timeout|" +
Expand Down