Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert quoting lookup fix. (#14034) 

* Revert "Add ANSI_QUOTES propety to DBI init in lookups. (#13826)"

This reverts commit 9e9976001c.

* Revert "Quote and escape literals in JDBC lookup to allow reserved identifiers. (#13632)"

This reverts commit 41fdf6eafb.

* fix typo.
This commit is contained in:
Abhishek Radhakrishnan 2023-04-05 20:52:36 -07:00 committed by GitHub
parent 5810e650d4
commit b98eed8fb8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java
View File

@ -19,7 +19,6 @@
package org.apache.druid.server.lookup.namespace; package org.apache.druid.server.lookup.namespace;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import org.apache.druid.data.input.MapPopulator; import org.apache.druid.data.input.MapPopulator;
import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.ISE;
@ -40,7 +39,6 @@ import org.skife.jdbi.v2.util.TimestampMapper;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
@ -162,29 +160,23 @@ public final class JdbcCacheGenerator implements CacheGenerator<JdbcExtractionNa
if (Strings.isNullOrEmpty(filter)) { if (Strings.isNullOrEmpty(filter)) {
return StringUtils.format( return StringUtils.format(
"SELECT %s, %s FROM %s WHERE %s IS NOT NULL", "SELECT %s, %s FROM %s WHERE %s IS NOT NULL",
toDoublyQuotedEscapedIdentifier(keyColumn), keyColumn,
toDoublyQuotedEscapedIdentifier(valueColumn), valueColumn,
toDoublyQuotedEscapedIdentifier(table), table,
toDoublyQuotedEscapedIdentifier(valueColumn) valueColumn
); );
} }
return StringUtils.format( return StringUtils.format(
"SELECT %s, %s FROM %s WHERE %s AND %s IS NOT NULL", "SELECT %s, %s FROM %s WHERE %s AND %s IS NOT NULL",
toDoublyQuotedEscapedIdentifier(keyColumn), keyColumn,
toDoublyQuotedEscapedIdentifier(valueColumn), valueColumn,
toDoublyQuotedEscapedIdentifier(table), table,
filter, filter,
toDoublyQuotedEscapedIdentifier(valueColumn) valueColumn
); );
} }
@VisibleForTesting
public static String toDoublyQuotedEscapedIdentifier(String identifier)
{
return "\"" + StringUtils.replace(identifier, "\"", "\"\"") + "\"";
}
private DBI ensureDBI(CacheScheduler.EntryImpl<JdbcExtractionNamespace> key, JdbcExtractionNamespace namespace) private DBI ensureDBI(CacheScheduler.EntryImpl<JdbcExtractionNamespace> key, JdbcExtractionNamespace namespace)
{ {
DBI dbi = null; DBI dbi = null;
@ -192,21 +184,10 @@ public final class JdbcCacheGenerator implements CacheGenerator<JdbcExtractionNa
dbi = dbiCache.get(key); dbi = dbiCache.get(key);
} }
if (dbi == null) { if (dbi == null) {
Properties props = new Properties();
if (namespace.getConnectorConfig().getUser() != null) {
props.setProperty("user", namespace.getConnectorConfig().getUser());
}
if (namespace.getConnectorConfig().getPassword() != null) {
props.setProperty("password", namespace.getConnectorConfig().getPassword());
}
// We use double quotes to quote identifiers. This enables us to write SQL
// that works with most databases that are SQL compliant.
props.setProperty("sessionVariables", "sql_mode='ANSI_QUOTES'");
final DBI newDbi = new DBI( final DBI newDbi = new DBI(
namespace.getConnectorConfig().getConnectURI(), namespace.getConnectorConfig().getConnectURI(),
props namespace.getConnectorConfig().getUser(),
namespace.getConnectorConfig().getPassword()
); );
dbiCache.putIfAbsent(key, newDbi); dbiCache.putIfAbsent(key, newDbi);
dbi = dbiCache.get(key); dbi = dbiCache.get(key);
@ -227,7 +208,7 @@ public final class JdbcCacheGenerator implements CacheGenerator<JdbcExtractionNa
handle -> { handle -> {
final String query = StringUtils.format( final String query = StringUtils.format(
"SELECT MAX(%s) FROM %s", "SELECT MAX(%s) FROM %s",
toDoublyQuotedEscapedIdentifier(tsColumn), toDoublyQuotedEscapedIdentifier(table) tsColumn, table
); );
return handle return handle
.createQuery(query) .createQuery(query)

78
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java vendored
View File

@ -77,7 +77,9 @@ public class JdbcExtractionNamespaceTest
public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule(); public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule();
private static final Logger log = new Logger(JdbcExtractionNamespaceTest.class); private static final Logger log = new Logger(JdbcExtractionNamespaceTest.class);
private static final String TABLE_NAME = "abstractDbRenameTest";
private static final String KEY_NAME = "keyName";
private static final String VAL_NAME = "valName";
private static final String TS_COLUMN = "tsColumn"; private static final String TS_COLUMN = "tsColumn";
private static final String FILTER_COLUMN = "filterColumn"; private static final String FILTER_COLUMN = "filterColumn";
private static final Map<String, String[]> RENAMES = ImmutableMap.of( private static final Map<String, String[]> RENAMES = ImmutableMap.of(
@ -88,32 +90,22 @@ public class JdbcExtractionNamespaceTest
); );
@Parameterized.Parameters(name = "tableName={0}, keyName={1}, valName={2}, tsColumn={3}") @Parameterized.Parameters(name = "{0}")
public static Collection<Object[]> getParameters() public static Collection<Object[]> getParameters()
{ {
return ImmutableList.of( return ImmutableList.of(
new Object[]{"table", "select", "foo \" column;", "tsColumn"}, // reserved identifiers as table, key and value columns. new Object[]{"tsColumn"},
new Object[]{"abstractDbRenameTest", "keyName", "valName", "tsColumn"}, new Object[]{null}
new Object[]{"abstractDbRenameTest", "keyName", "valName", null}
); );
} }
public JdbcExtractionNamespaceTest( public JdbcExtractionNamespaceTest(
String tableName,
String keyName,
String valName,
String tsColumn String tsColumn
) )
{ {
this.tableName = tableName;
this.keyName = keyName;
this.valName = valName;
this.tsColumn = tsColumn; this.tsColumn = tsColumn;
} }
private final String tableName;
private final String keyName;
private final String valName;
private final String tsColumn; private final String tsColumn;
private CacheScheduler scheduler; private CacheScheduler scheduler;
private Lifecycle lifecycle; private Lifecycle lifecycle;
@ -140,20 +132,18 @@ public class JdbcExtractionNamespaceTest
handle.createStatement( handle.createStatement(
StringUtils.format( StringUtils.format(
"CREATE TABLE %s (%s TIMESTAMP, %s VARCHAR(64), %s VARCHAR(64), %s VARCHAR(64))", "CREATE TABLE %s (%s TIMESTAMP, %s VARCHAR(64), %s VARCHAR(64), %s VARCHAR(64))",
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), TABLE_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(TS_COLUMN), TS_COLUMN,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN), FILTER_COLUMN,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), KEY_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName) VAL_NAME
) )
).setQueryTimeout(1).execute() ).setQueryTimeout(1).execute()
); );
handle.createStatement(StringUtils.format("TRUNCATE TABLE %s", handle.createStatement(StringUtils.format("TRUNCATE TABLE %s", TABLE_NAME)).setQueryTimeout(1).execute();
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName))).setQueryTimeout(1).execute();
handle.commit(); handle.commit();
closer.register(() -> { closer.register(() -> {
handle.createStatement(StringUtils.format("DROP TABLE %s", handle.createStatement("DROP TABLE " + TABLE_NAME).setQueryTimeout(1).execute();
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName))).setQueryTimeout(1).execute();
final ListenableFuture future = setupTeardownService.submit(new Runnable() final ListenableFuture future = setupTeardownService.submit(new Runnable()
{ {
@Override @Override
@ -302,25 +292,19 @@ public class JdbcExtractionNamespaceTest
final String statementVal = val != null ? "'%s'" : "%s"; final String statementVal = val != null ? "'%s'" : "%s";
if (tsColumn == null) { if (tsColumn == null) {
handle.createStatement( handle.createStatement(
StringUtils.format("DELETE FROM %s WHERE %s='%s'", JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), StringUtils.format("DELETE FROM %s WHERE %s='%s'", TABLE_NAME, KEY_NAME, key)
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), key)
).setQueryTimeout(1).execute(); ).setQueryTimeout(1).execute();
query = StringUtils.format( query = StringUtils.format(
"INSERT INTO %s (%s, %s, %s) VALUES ('%s', '%s', " + statementVal + ")", "INSERT INTO %s (%s, %s, %s) VALUES ('%s', '%s', " + statementVal + ")",
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), TABLE_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN), FILTER_COLUMN, KEY_NAME, VAL_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName),
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName),
filter, key, val filter, key, val
); );
} else { } else {
query = StringUtils.format( query = StringUtils.format(
"INSERT INTO %s (%s, %s, %s, %s) VALUES ('%s', '%s', '%s', " + statementVal + ")", "INSERT INTO %s (%s, %s, %s, %s) VALUES ('%s', '%s', '%s', " + statementVal + ")",
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), TABLE_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tsColumn), tsColumn, FILTER_COLUMN, KEY_NAME, VAL_NAME,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN),
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName),
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName),
updateTs, filter, key, val updateTs, filter, key, val
); );
} }
@ -337,9 +321,9 @@ public class JdbcExtractionNamespaceTest
{ {
final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace(
derbyConnectorRule.getMetadataConnectorConfig(), derbyConnectorRule.getMetadataConnectorConfig(),
tableName, TABLE_NAME,
keyName, KEY_NAME,
valName, VAL_NAME,
tsColumn, tsColumn,
null, null,
new Period(0), new Period(0),
@ -370,11 +354,11 @@ public class JdbcExtractionNamespaceTest
{ {
final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace(
derbyConnectorRule.getMetadataConnectorConfig(), derbyConnectorRule.getMetadataConnectorConfig(),
tableName, TABLE_NAME,
keyName, KEY_NAME,
valName, VAL_NAME,
tsColumn, tsColumn,
JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN) + "='1'", FILTER_COLUMN + "='1'",
new Period(0), new Period(0),
null, null,
new JdbcAccessSecurityConfig() new JdbcAccessSecurityConfig()
@ -445,9 +429,9 @@ public class JdbcExtractionNamespaceTest
final JdbcAccessSecurityConfig securityConfig = new JdbcAccessSecurityConfig(); final JdbcAccessSecurityConfig securityConfig = new JdbcAccessSecurityConfig();
final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace(
derbyConnectorRule.getMetadataConnectorConfig(), derbyConnectorRule.getMetadataConnectorConfig(),
tableName, TABLE_NAME,
keyName, KEY_NAME,
valName, VAL_NAME,
tsColumn, tsColumn,
"some filter", "some filter",
new Period(10), new Period(10),
@ -470,9 +454,9 @@ public class JdbcExtractionNamespaceTest
{ {
final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace(
derbyConnectorRule.getMetadataConnectorConfig(), derbyConnectorRule.getMetadataConnectorConfig(),
tableName, TABLE_NAME,
keyName, KEY_NAME,
valName, VAL_NAME,
tsColumn, tsColumn,
null, null,
new Period(10), new Period(10),