From b98eed8fb83e3868fdd88c6edff696bf0290cf15 Mon Sep 17 00:00:00 2001 From: Abhishek Radhakrishnan Date: Wed, 5 Apr 2023 20:52:36 -0700 Subject: [PATCH] Revert quoting lookup fix. (#14034) * Revert "Add ANSI_QUOTES propety to DBI init in lookups. (#13826)" This reverts commit 9e9976001c5692732be4b7e28d79886e0d6859a9. * Revert "Quote and escape literals in JDBC lookup to allow reserved identifiers. (#13632)" This reverts commit 41fdf6eafbf3ff4bb67909ba15a2eaeb648dd036. * fix typo. --- .../lookup/namespace/JdbcCacheGenerator.java | 41 +++------- .../cache/JdbcExtractionNamespaceTest.java | 78 ++++++++----------- 2 files changed, 42 insertions(+), 77 deletions(-) diff --git a/extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java b/extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java index 8c0fc5f7db8..f05ff702504 100644 --- a/extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java +++ b/extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java @@ -19,7 +19,6 @@ package org.apache.druid.server.lookup.namespace; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import org.apache.druid.data.input.MapPopulator; import org.apache.druid.java.util.common.ISE; @@ -40,7 +39,6 @@ import org.skife.jdbi.v2.util.TimestampMapper; import javax.annotation.Nullable; import java.sql.Timestamp; -import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -162,29 +160,23 @@ public final class JdbcCacheGenerator implements CacheGenerator key, JdbcExtractionNamespace namespace) { DBI dbi = null; @@ -192,21 +184,10 @@ public final class JdbcCacheGenerator implements CacheGenerator { final String query = StringUtils.format( "SELECT MAX(%s) FROM %s", - toDoublyQuotedEscapedIdentifier(tsColumn), toDoublyQuotedEscapedIdentifier(table) + tsColumn, table ); return handle .createQuery(query) diff --git a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java index 3e219fce075..b6a37240cea 100644 --- a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java +++ b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java @@ -77,7 +77,9 @@ public class JdbcExtractionNamespaceTest public final TestDerbyConnector.DerbyConnectorRule derbyConnectorRule = new TestDerbyConnector.DerbyConnectorRule(); 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 FILTER_COLUMN = "filterColumn"; private static final Map 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 getParameters() { return ImmutableList.of( - new Object[]{"table", "select", "foo \" column;", "tsColumn"}, // reserved identifiers as table, key and value columns. - new Object[]{"abstractDbRenameTest", "keyName", "valName", "tsColumn"}, - new Object[]{"abstractDbRenameTest", "keyName", "valName", null} + new Object[]{"tsColumn"}, + new Object[]{null} ); } public JdbcExtractionNamespaceTest( - String tableName, - String keyName, - String valName, String tsColumn ) { - this.tableName = tableName; - this.keyName = keyName; - this.valName = valName; this.tsColumn = tsColumn; } - private final String tableName; - private final String keyName; - private final String valName; private final String tsColumn; private CacheScheduler scheduler; private Lifecycle lifecycle; @@ -140,20 +132,18 @@ public class JdbcExtractionNamespaceTest handle.createStatement( StringUtils.format( "CREATE TABLE %s (%s TIMESTAMP, %s VARCHAR(64), %s VARCHAR(64), %s VARCHAR(64))", - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(TS_COLUMN), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName) + TABLE_NAME, + TS_COLUMN, + FILTER_COLUMN, + KEY_NAME, + VAL_NAME ) ).setQueryTimeout(1).execute() ); - handle.createStatement(StringUtils.format("TRUNCATE TABLE %s", - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName))).setQueryTimeout(1).execute(); + handle.createStatement(StringUtils.format("TRUNCATE TABLE %s", TABLE_NAME)).setQueryTimeout(1).execute(); handle.commit(); closer.register(() -> { - handle.createStatement(StringUtils.format("DROP TABLE %s", - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName))).setQueryTimeout(1).execute(); + handle.createStatement("DROP TABLE " + TABLE_NAME).setQueryTimeout(1).execute(); final ListenableFuture future = setupTeardownService.submit(new Runnable() { @Override @@ -302,25 +292,19 @@ public class JdbcExtractionNamespaceTest final String statementVal = val != null ? "'%s'" : "%s"; if (tsColumn == null) { handle.createStatement( - StringUtils.format("DELETE FROM %s WHERE %s='%s'", JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), key) + StringUtils.format("DELETE FROM %s WHERE %s='%s'", TABLE_NAME, KEY_NAME, key) ).setQueryTimeout(1).execute(); query = StringUtils.format( "INSERT INTO %s (%s, %s, %s) VALUES ('%s', '%s', " + statementVal + ")", - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName), + TABLE_NAME, + FILTER_COLUMN, KEY_NAME, VAL_NAME, filter, key, val ); } else { query = StringUtils.format( "INSERT INTO %s (%s, %s, %s, %s) VALUES ('%s', '%s', '%s', " + statementVal + ")", - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tableName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(tsColumn), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(keyName), - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(valName), + TABLE_NAME, + tsColumn, FILTER_COLUMN, KEY_NAME, VAL_NAME, updateTs, filter, key, val ); } @@ -337,9 +321,9 @@ public class JdbcExtractionNamespaceTest { final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( derbyConnectorRule.getMetadataConnectorConfig(), - tableName, - keyName, - valName, + TABLE_NAME, + KEY_NAME, + VAL_NAME, tsColumn, null, new Period(0), @@ -370,11 +354,11 @@ public class JdbcExtractionNamespaceTest { final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( derbyConnectorRule.getMetadataConnectorConfig(), - tableName, - keyName, - valName, + TABLE_NAME, + KEY_NAME, + VAL_NAME, tsColumn, - JdbcCacheGenerator.toDoublyQuotedEscapedIdentifier(FILTER_COLUMN) + "='1'", + FILTER_COLUMN + "='1'", new Period(0), null, new JdbcAccessSecurityConfig() @@ -445,9 +429,9 @@ public class JdbcExtractionNamespaceTest final JdbcAccessSecurityConfig securityConfig = new JdbcAccessSecurityConfig(); final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( derbyConnectorRule.getMetadataConnectorConfig(), - tableName, - keyName, - valName, + TABLE_NAME, + KEY_NAME, + VAL_NAME, tsColumn, "some filter", new Period(10), @@ -470,9 +454,9 @@ public class JdbcExtractionNamespaceTest { final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( derbyConnectorRule.getMetadataConnectorConfig(), - tableName, - keyName, - valName, + TABLE_NAME, + KEY_NAME, + VAL_NAME, tsColumn, null, new Period(10),