Skip to content

Allow implicit escape for _ but not % in JDBC metadata calls #12672

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 2 commits into from
Jun 9, 2022
Merged

Allow implicit escape for _ but not % in JDBC metadata calls #12672

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
d098bb4
Add more whitespace in JDBC parameter reference table
hovaesco Jun 8, 2022
cef33e4
Allow implicit escape for _ but not % in JDBC metadata calls
hovaesco Jun 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
29 changes: 28 additions & 1 deletion 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 @@ -59,6 +59,7 @@ enum SslVerificationMode
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> ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS = new AssumeLiteralUnderscoreInMetadataCallsForNonConformingClients();
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 @@ -98,6 +99,7 @@ enum SslVerificationMode
.add(APPLICATION_NAME_PREFIX)
.add(DISABLE_COMPRESSION)
.add(ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS)
.add(ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS)
.add(SSL)
.add(SSL_VERIFICATION)
.add(SSL_KEY_STORE_PATH)
Expand Down Expand Up @@ -289,9 +291,34 @@ public DisableCompression()
private static class AssumeLiteralNamesInMetadataCallsForNonConformingClients
extends AbstractConnectionProperty<Boolean>
{
private static final Predicate<Properties> IS_ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED =
checkedPredicate(properties -> !ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS.getValue(properties).orElse(false));

public AssumeLiteralNamesInMetadataCallsForNonConformingClients()
{
super("assumeLiteralNamesInMetadataCallsForNonConformingClients", NOT_REQUIRED, ALLOWED, BOOLEAN_CONVERTER);
super(
"assumeLiteralNamesInMetadataCallsForNonConformingClients",
NOT_REQUIRED,
AssumeLiteralUnderscoreInMetadataCallsForNonConformingClients.IS_ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED
.or(IS_ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED),
BOOLEAN_CONVERTER);
}
}

private static class AssumeLiteralUnderscoreInMetadataCallsForNonConformingClients
extends AbstractConnectionProperty<Boolean>
{
private static final Predicate<Properties> IS_ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED =
checkedPredicate(properties -> !ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS.getValue(properties).orElse(false));

public AssumeLiteralUnderscoreInMetadataCallsForNonConformingClients()
{
super(
"assumeLiteralUnderscoreInMetadataCallsForNonConformingClients",
NOT_REQUIRED,
AssumeLiteralNamesInMetadataCallsForNonConformingClients.IS_ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED
.or(IS_ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS_NOT_ENABLED),
BOOLEAN_CONVERTER);
}
}

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 @@ -97,6 +97,7 @@ public class TrinoConnection
private final Optional<String> sessionUser;
private final boolean compressionDisabled;
private final boolean assumeLiteralNamesInMetadataCallsForNonConformingClients;
private final boolean assumeLiteralUnderscoreInMetadataCallsForNonConformingClients;
private final Map<String, String> extraCredentials;
private final Optional<String> applicationNamePrefix;
private final Optional<String> source;
Expand All @@ -123,6 +124,7 @@ public class TrinoConnection
this.extraCredentials = uri.getExtraCredentials();
this.compressionDisabled = uri.isCompressionDisabled();
this.assumeLiteralNamesInMetadataCallsForNonConformingClients = uri.isAssumeLiteralNamesInMetadataCallsForNonConformingClients();
this.assumeLiteralUnderscoreInMetadataCallsForNonConformingClients = uri.isAssumeLiteralUnderscoreInMetadataCallsForNonConformingClients();
this.httpClient = requireNonNull(httpClient, "httpClient is null");
uri.getClientInfo().ifPresent(tags -> clientInfo.put(CLIENT_INFO, tags));
uri.getClientTags().ifPresent(tags -> clientInfo.put(CLIENT_TAGS, tags));
Expand Down Expand Up @@ -271,7 +273,7 @@ public boolean isClosed()
public DatabaseMetaData getMetaData()
throws SQLException
{
return new TrinoDatabaseMetaData(this, assumeLiteralNamesInMetadataCallsForNonConformingClients);
return new TrinoDatabaseMetaData(this, assumeLiteralNamesInMetadataCallsForNonConformingClients, assumeLiteralUnderscoreInMetadataCallsForNonConformingClients);
}

@Override
Expand Down
25 changes: 19 additions & 6 deletions 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 @@ -34,6 +34,7 @@
import java.util.List;
import java.util.stream.Stream;

import static com.google.common.base.Preconditions.checkArgument;
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;
Expand All @@ -51,11 +52,16 @@ public class TrinoDatabaseMetaData

private final TrinoConnection connection;
private final boolean assumeLiteralNamesInMetadataCallsForNonConformingClients;
private final boolean assumeLiteralUnderscoreInMetadataCallsForNonConformingClients;

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

@Override
Expand Down Expand Up @@ -1563,19 +1569,26 @@ private static void optionalStringInFilter(List<String> filters, String columnNa

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

@Nullable
static String escapeIfNecessary(boolean assumeLiteralNamesInMetadataCallsForNonConformingClients, @Nullable String namePattern)
{
if (namePattern == null || !assumeLiteralNamesInMetadataCallsForNonConformingClients) {
static String escapeIfNecessary(
boolean assumeLiteralNamesInMetadataCallsForNonConformingClients,
boolean assumeLiteralUnderscoreInMetadataCallsForNonConformingClients,
@Nullable String namePattern)
{
checkArgument(
!assumeLiteralNamesInMetadataCallsForNonConformingClients || !assumeLiteralUnderscoreInMetadataCallsForNonConformingClients,
"assumeLiteralNamesInMetadataCallsForNonConformingClients and assumeLiteralUnderscoreInMetadataCallsForNonConformingClients cannot be both true");
if (namePattern == null || (!assumeLiteralNamesInMetadataCallsForNonConformingClients && !assumeLiteralUnderscoreInMetadataCallsForNonConformingClients)) {
return namePattern;
}
//noinspection ConstantConditions
verify(SEARCH_STRING_ESCAPE.equals("\\"));
return namePattern.replaceAll("[_%\\\\]", "\\\\$0");
return namePattern.replaceAll(assumeLiteralNamesInMetadataCallsForNonConformingClients ? "[_%\\\\]" : "[_\\\\]", "\\\\$0");
}

private static void optionalStringLikeFilter(List<String> filters, String columnName, String value)
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 @@ -54,6 +54,7 @@
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.ASSUME_LITERAL_UNDERSCORE_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 @@ -251,6 +252,12 @@ public boolean isAssumeLiteralNamesInMetadataCallsForNonConformingClients()
return ASSUME_LITERAL_NAMES_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS.getValue(properties).orElse(false);
}

public boolean isAssumeLiteralUnderscoreInMetadataCallsForNonConformingClients()
throws SQLException
{
return ASSUME_LITERAL_UNDERSCORE_IN_METADATA_CALLS_FOR_NON_CONFORMING_CLIENTS.getValue(properties).orElse(false);
}

public void setupClient(OkHttpClient.Builder builder)
throws SQLException
{
Expand Down
64 changes: 48 additions & 16 deletions client/trino-jdbc/src/test/java/io/trino/jdbc/TestTrinoDatabaseMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.sql.Connection;
Expand Down Expand Up @@ -86,6 +87,7 @@
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
Expand Down Expand Up @@ -1419,12 +1421,12 @@ public void testGetColumnsMetadataCalls()
.withGetColumnsCount(3000));
}

@Test
public void testAssumeLiteralMetadataCalls()
@Test(dataProvider = "escapeLiteralParameters")
public void testAssumeLiteralMetadataCalls(String escapeLiteralParameter)
throws Exception
{
try (Connection connection = DriverManager.getConnection(
format("jdbc:trino://%s?assumeLiteralNamesInMetadataCallsForNonConformingClients=true", server.getAddress()),
format("jdbc:trino://%s?%s", server.getAddress(), escapeLiteralParameter),
"admin",
null)) {
// getTables's schema name pattern treated as literal
Expand Down Expand Up @@ -1497,22 +1499,52 @@ public void testAssumeLiteralMetadataCalls()
}
}

@DataProvider
public Object[][] escapeLiteralParameters()
{
return new Object[][]{
{"assumeLiteralNamesInMetadataCallsForNonConformingClients=true"},
{"assumeLiteralUnderscoreInMetadataCallsForNonConformingClients=true"},
{"assumeLiteralNamesInMetadataCallsForNonConformingClients=false&assumeLiteralUnderscoreInMetadataCallsForNonConformingClients=true"},
{"assumeLiteralNamesInMetadataCallsForNonConformingClients=true&assumeLiteralUnderscoreInMetadataCallsForNonConformingClients=false"},
};
}

@Test
public void testFailedBothEscapeLiteralParameters()
throws SQLException
{
assertThatThrownBy(() -> DriverManager.getConnection(
format("jdbc:trino://%s?%s", server.getAddress(), "assumeLiteralNamesInMetadataCallsForNonConformingClients=true&assumeLiteralUnderscoreInMetadataCallsForNonConformingClients=true"),
"admin",
null))
.isInstanceOf(SQLException.class)
.hasMessage("Connection property 'assumeLiteralNamesInMetadataCallsForNonConformingClients' is not allowed");
}

@Test
public void testEscapeIfNecessary()
throws SQLException
{
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, null), null);
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, "a"), "a");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, "abc_def"), "abc_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, "abc__de_f"), "abc__de_f");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, "abc%def"), "abc%def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, "abc\\_def"), "abc\\_def");

assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, null), null);
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, "a"), "a");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, "abc_def"), "abc\\_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, "abc__de_f"), "abc\\_\\_de\\_f");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, "abc%def"), "abc\\%def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, "abc\\_def"), "abc\\\\\\_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, null), null);
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, "a"), "a");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, "abc_def"), "abc_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, "abc__de_f"), "abc__de_f");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, "abc%def"), "abc%def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, false, "abc\\_def"), "abc\\_def");

assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, null), null);
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, "a"), "a");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, "abc_def"), "abc\\_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, "abc__de_f"), "abc\\_\\_de\\_f");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, "abc%def"), "abc\\%def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(true, false, "abc\\_def"), "abc\\\\\\_def");

assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, true, null), null);
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, true, "a"), "a");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, true, "abc_def"), "abc\\_def");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, true, "abc__de_f"), "abc\\_\\_de\\_f");
assertEquals(TrinoDatabaseMetaData.escapeIfNecessary(false, true, "abc\\_def"), "abc\\\\\\_def");
}

@Test
Expand Down
Loading