Skip to content

Add compatibility with misbehaving client applications #7438

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
findepi merged 4 commits into from
Mar 30, 2021
Merged

Add compatibility with misbehaving client applications #7438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0eba2d
Remove redundant suppression
findepi Mar 30, 2021
caced61
Use existing connection in assertMetadataCalls
findepi Mar 30, 2021
9c34eb9
Count getTableHandle calls in metadata calls tests
findepi Mar 30, 2021
69778e1
Add compatibility with misbehaving client applications
findepi Mar 26, 2021
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
11 changes: 11 additions & 0 deletions client/trino-jdbc/src/main/java/io/trino/jdbc/ConnectionProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ enum SslVerificationMode
public static final ConnectionProperty<HostAndPort> HTTP_PROXY = new HttpProxy();
public static final ConnectionProperty<String> APPLICATION_NAME_PREFIX = new ApplicationNamePrefix();
public static final ConnectionProperty<Boolean> DISABLE_COMPRESSION = new DisableCompression();
public static final ConnectionProperty<Boolean> ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS = new AssumeLiteralNamesInMetadataCallsForNonConformingClients();
public static final ConnectionProperty<Boolean> SSL = new Ssl();
public static final ConnectionProperty<SslVerificationMode> SSL_VERIFICATION = new SslVerification();
public static final ConnectionProperty<String> SSL_KEY_STORE_PATH = new SslKeyStorePath();
Expand Down Expand Up @@ -89,6 +90,7 @@ enum SslVerificationMode
.add(HTTP_PROXY)
.add(APPLICATION_NAME_PREFIX)
.add(DISABLE_COMPRESSION)
.add(ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS)
.add(SSL)
.add(SSL_VERIFICATION)
.add(SSL_KEY_STORE_PATH)
Expand Down Expand Up @@ -273,6 +275,15 @@ public DisableCompression()
}
}

private static class AssumeLiteralNamesInMetadataCallsForNonConformingClients
extends AbstractConnectionProperty<Boolean>
{
public AssumeLiteralNamesInMetadataCallsForNonConformingClients()
{
super("assumeLiteralNamesInMetadataCallsForNonConformingClients", NOT_REQUIRED, ALLOWED, BOOLEAN_CONVERTER);
}
}

private static class Ssl
extends AbstractConnectionProperty<Boolean>
{
Expand Down
4 changes: 3 additions & 1 deletion client/trino-jdbc/src/main/java/io/trino/jdbc/TrinoConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public class TrinoConnection
private final String user;
private final Optional<String> sessionUser;
private final boolean compressionDisabled;
private final boolean assumeLiteralNamesInMetadataCallsForNonConformingClients;
private final Map<String, String> extraCredentials;
private final Optional<String> applicationNamePrefix;
private final Optional<String> source;
Expand All @@ -115,6 +116,7 @@ public class TrinoConnection
this.source = uri.getSource();
this.extraCredentials = uri.getExtraCredentials();
this.compressionDisabled = uri.isCompressionDisabled();
this.assumeLiteralNamesInMetadataCallsForNonConformingClients = uri.isAssumeLiteralNamesInMetadataCallsForNonConformingClients();
this.queryExecutor = requireNonNull(queryExecutor, "queryExecutor is null");
uri.getClientInfo().ifPresent(tags -> clientInfo.put(CLIENT_INFO, tags));
uri.getClientTags().ifPresent(tags -> clientInfo.put(CLIENT_TAGS, tags));
Expand Down Expand Up @@ -238,7 +240,7 @@ public boolean isClosed()
public DatabaseMetaData getMetaData()
throws SQLException
{
return new TrinoDatabaseMetaData(this);
return new TrinoDatabaseMetaData(this, assumeLiteralNamesInMetadataCallsForNonConformingClients);
}

@Override
Expand Down
55 changes: 54 additions & 1 deletion client/trino-jdbc/src/main/java/io/trino/jdbc/TrinoDatabaseMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import io.trino.client.ClientTypeSignatureParameter;
import io.trino.client.Column;

import javax.annotation.Nullable;

import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
Expand All @@ -31,6 +33,7 @@
import java.util.List;
import java.util.stream.Stream;

import static com.google.common.base.Verify.verify;
import static com.google.common.collect.Lists.newArrayList;
import static io.trino.client.ClientTypeSignature.VARCHAR_UNBOUNDED_LENGTH;
import static io.trino.jdbc.DriverInfo.DRIVER_NAME;
Expand All @@ -46,10 +49,12 @@ public class TrinoDatabaseMetaData
private static final String SEARCH_STRING_ESCAPE = "\\";

private final TrinoConnection connection;
private final boolean assumeLiteralNamesInMetadataCallsForNonConformingClients;

TrinoDatabaseMetaData(TrinoConnection connection)
TrinoDatabaseMetaData(TrinoConnection connection, boolean assumeLiteralNamesInMetadataCallsForNonConformingClients)
{
this.connection = requireNonNull(connection, "connection is null");
this.assumeLiteralNamesInMetadataCallsForNonConformingClients = assumeLiteralNamesInMetadataCallsForNonConformingClients;
}

@Override
Expand Down Expand Up @@ -897,6 +902,8 @@ public boolean dataDefinitionIgnoredInTransactions()
public ResultSet getProcedures(String catalog, String schemaPattern, String procedureNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
procedureNamePattern = escapeIfNecessary(procedureNamePattern);
return selectEmpty("" +
"SELECT PROCEDURE_CAT, PROCEDURE_SCHEM, PROCEDURE_NAME,\n " +
" null, null, null, REMARKS, PROCEDURE_TYPE, SPECIFIC_NAME\n" +
Expand All @@ -908,6 +915,9 @@ public ResultSet getProcedures(String catalog, String schemaPattern, String proc
public ResultSet getProcedureColumns(String catalog, String schemaPattern, String procedureNamePattern, String columnNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
procedureNamePattern = escapeIfNecessary(procedureNamePattern);
columnNamePattern = escapeIfNecessary(columnNamePattern);
return selectEmpty("" +
"SELECT PROCEDURE_CAT, PROCEDURE_SCHEM, PROCEDURE_NAME, " +
" COLUMN_NAME, COLUMN_TYPE, DATA_TYPE, TYPE_NAME,\n" +
Expand All @@ -922,6 +932,8 @@ public ResultSet getProcedureColumns(String catalog, String schemaPattern, Strin
public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
tableNamePattern = escapeIfNecessary(tableNamePattern);
StringBuilder query = new StringBuilder("" +
"SELECT TABLE_CAT, TABLE_SCHEM, TABLE_NAME, TABLE_TYPE, REMARKS,\n" +
" TYPE_CAT, TYPE_SCHEM, TYPE_NAME, " +
Expand Down Expand Up @@ -974,6 +986,9 @@ public ResultSet getTableTypes()
public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
tableNamePattern = escapeIfNecessary(tableNamePattern);
columnNamePattern = escapeIfNecessary(columnNamePattern);
StringBuilder query = new StringBuilder("" +
"SELECT TABLE_CAT, TABLE_SCHEM, TABLE_NAME, COLUMN_NAME, DATA_TYPE,\n" +
" TYPE_NAME, COLUMN_SIZE, BUFFER_LENGTH, DECIMAL_DIGITS, NUM_PREC_RADIX,\n" +
Expand All @@ -999,13 +1014,16 @@ public ResultSet getColumns(String catalog, String schemaPattern, String tableNa
public ResultSet getColumnPrivileges(String catalog, String schema, String table, String columnNamePattern)
throws SQLException
{
columnNamePattern = escapeIfNecessary(columnNamePattern);
throw new SQLFeatureNotSupportedException("privileges not supported");
}

@Override
public ResultSet getTablePrivileges(String catalog, String schemaPattern, String tableNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
tableNamePattern = escapeIfNecessary(tableNamePattern);
throw new SQLFeatureNotSupportedException("privileges not supported");
}

Expand Down Expand Up @@ -1168,6 +1186,8 @@ public boolean supportsBatchUpdates()
public ResultSet getUDTs(String catalog, String schemaPattern, String typeNamePattern, int[] types)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
typeNamePattern = escapeIfNecessary(typeNamePattern);
return selectEmpty("" +
"SELECT TYPE_CAT, TYPE_SCHEM, TYPE_NAME,\n" +
" CLASS_NAME, DATA_TYPE, REMARKS, BASE_TYPE\n" +
Expand Down Expand Up @@ -1214,6 +1234,8 @@ public boolean supportsGetGeneratedKeys()
public ResultSet getSuperTypes(String catalog, String schemaPattern, String typeNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
typeNamePattern = escapeIfNecessary(typeNamePattern);
return selectEmpty("" +
"SELECT TYPE_CAT, TYPE_SCHEM, TYPE_NAME,\n" +
" SUPERTYPE_CAT, SUPERTYPE_SCHEM, SUPERTYPE_NAME\n" +
Expand All @@ -1225,6 +1247,8 @@ public ResultSet getSuperTypes(String catalog, String schemaPattern, String type
public ResultSet getSuperTables(String catalog, String schemaPattern, String tableNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
tableNamePattern = escapeIfNecessary(tableNamePattern);
return selectEmpty("" +
"SELECT TABLE_CAT, TABLE_SCHEM, TABLE_NAME, SUPERTABLE_NAME\n" +
"FROM system.jdbc.super_tables\n" +
Expand All @@ -1235,6 +1259,9 @@ public ResultSet getSuperTables(String catalog, String schemaPattern, String tab
public ResultSet getAttributes(String catalog, String schemaPattern, String typeNamePattern, String attributeNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
typeNamePattern = escapeIfNecessary(typeNamePattern);
attributeNamePattern = escapeIfNecessary(attributeNamePattern);
return selectEmpty("" +
"SELECT TYPE_CAT, TYPE_SCHEM, TYPE_NAME, ATTR_NAME, DATA_TYPE,\n" +
" ATTR_TYPE_NAME, ATTR_SIZE, DECIMAL_DIGITS, NUM_PREC_RADIX, NULLABLE,\n" +
Expand Down Expand Up @@ -1332,6 +1359,7 @@ public RowIdLifetime getRowIdLifetime()
public ResultSet getSchemas(String catalog, String schemaPattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
StringBuilder query = new StringBuilder("" +
"SELECT TABLE_SCHEM, TABLE_CATALOG\n" +
"FROM system.jdbc.schemas");
Expand Down Expand Up @@ -1391,6 +1419,8 @@ public ResultSet getClientInfoProperties()
public ResultSet getFunctions(String catalog, String schemaPattern, String functionNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
functionNamePattern = escapeIfNecessary(functionNamePattern);
// TODO: implement this
throw new NotImplementedException("DatabaseMetaData", "getFunctions");
}
Expand All @@ -1399,6 +1429,9 @@ public ResultSet getFunctions(String catalog, String schemaPattern, String funct
public ResultSet getFunctionColumns(String catalog, String schemaPattern, String functionNamePattern, String columnNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
functionNamePattern = escapeIfNecessary(functionNamePattern);
columnNamePattern = escapeIfNecessary(columnNamePattern);
// TODO: implement this
throw new NotImplementedException("DatabaseMetaData", "getFunctionColumns");
}
Expand All @@ -1407,6 +1440,9 @@ public ResultSet getFunctionColumns(String catalog, String schemaPattern, String
public ResultSet getPseudoColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern)
throws SQLException
{
schemaPattern = escapeIfNecessary(schemaPattern);
tableNamePattern = escapeIfNecessary(tableNamePattern);
columnNamePattern = escapeIfNecessary(columnNamePattern);
return selectEmpty("" +
"SELECT TABLE_CAT, TABLE_SCHEM, TABLE_NAME, COLUMN_NAME, DATA_TYPE,\n" +
" COLUMN_SIZE, DECIMAL_DIGITS, NUM_PREC_RADIX, COLUMN_USAGE, REMARKS,\n" +
Expand Down Expand Up @@ -1485,6 +1521,23 @@ private static void optionalStringInFilter(List<String> filters, String columnNa
filters.add(filter.toString());
}

@Nullable
private String escapeIfNecessary(@Nullable String namePattern)
{
return escapeIfNecessary(assumeLiteralNamesInMetadataCallsForNonConformingClients, namePattern);
}

@Nullable
static String escapeIfNecessary(boolean assumeLiteralNamesInMetadataCallsForNonConformingClients, @Nullable String namePattern)
{
if (namePattern == null || !assumeLiteralNamesInMetadataCallsForNonConformingClients) {
return namePattern;
}
//noinspection ConstantConditions
verify(SEARCH_STRING_ESCAPE.equals("\\"));
Copy link
Copy Markdown
Member

@losipiuk losipiuk Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given fact we have no test for new functionality we will only notice this verify fails on deployed Trino, if constant value changes. A simple unit test for this method would help.

return namePattern.replaceAll("[_%\\\\]", "\\\\$0");
}

private static void optionalStringLikeFilter(List<String> filters, String columnName, String value)
{
if (value != null) {
Expand Down
7 changes: 7 additions & 0 deletions client/trino-jdbc/src/main/java/io/trino/jdbc/TrinoDriverUri.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import static io.trino.client.OkHttpUtil.tokenAuth;
import static io.trino.jdbc.ConnectionProperties.ACCESS_TOKEN;
import static io.trino.jdbc.ConnectionProperties.APPLICATION_NAME_PREFIX;
import static io.trino.jdbc.ConnectionProperties.ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS;
import static io.trino.jdbc.ConnectionProperties.CLIENT_INFO;
import static io.trino.jdbc.ConnectionProperties.CLIENT_TAGS;
import static io.trino.jdbc.ConnectionProperties.DISABLE_COMPRESSION;
Expand Down Expand Up @@ -236,6 +237,12 @@ public boolean isCompressionDisabled()
return DISABLE_COMPRESSION.getValue(properties).orElse(false);
}

public boolean isAssumeLiteralNamesInMetadataCallsForNonConformingClients()
throws SQLException
{
return ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS.getValue(properties).orElse(false);
}

public void setupClient(OkHttpClient.Builder builder)
throws SQLException
{
Expand Down
Loading