This is an automated email from the ASF dual-hosted git repository. vihangk1 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0f85cbd51130bb4ec3e7db9bdeb10e4ffe3470ae Author: Aman Sinha <[email protected]> AuthorDate: Sun Apr 5 11:58:35 2020 -0700 IMPALA-9602: Fix case-sensitivity for local catalog This patch makes the database and table names lower-case when doing lookups, insertion and invalidations of database objects or table objects in the local catalog cache. The remote catalog already does the right thing by lower-casing these names, so this patch makes the behavior consistent with what the remote catalog does. Testing: - Added unit tests for CatalogdMetaProvider by examining cache hits and misses when loading and invalidating database or tables with upper-case names. - Manually tested as follows: start Impala with local catalog enabled start-impala-cluster.py --catalogd_args="--catalog_topic_mode=minimal" --impalad_args="--use_local_catalog=true" Create database in lower-case: "CREATE DATABASE db1;" Run the following a few times (this errors without the patch): impala-shell.sh -q "DROP TABLE IF EXISTS DB1.ddl_test1 PURGE; CREATE TABLE DB1.ddl_test1 (val string);" Change-Id: I3f368fa9b50e22ec5057d0bf66c3fd51064d4c26 Reviewed-on: http://gerrit.cloudera.org:8080/15653 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../impala/catalog/local/CatalogdMetaProvider.java | 14 +++-- .../catalog/local/CatalogdMetaProviderTest.java | 59 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java index 618c2e9..2c8ece4 100644 --- a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java +++ b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java @@ -622,7 +622,7 @@ public class CatalogdMetaProvider implements MetaProvider { public Database loadDb(final String dbName) throws TException { return loadWithCaching("database metadata for " + dbName, DB_METADATA_STATS_CATEGORY, - new DbCacheKey(dbName, DbCacheKey.DbInfoType.HMS_METADATA), + new DbCacheKey(dbName.toLowerCase(), DbCacheKey.DbInfoType.HMS_METADATA), new Callable<Database>() { @Override public Database call() throws Exception { @@ -641,7 +641,7 @@ public class CatalogdMetaProvider implements MetaProvider { throws MetaException, UnknownDBException, TException { return loadWithCaching("table names for database " + dbName, TABLE_NAMES_STATS_CATEGORY, - new DbCacheKey(dbName, DbCacheKey.DbInfoType.TABLE_NAMES), + new DbCacheKey(dbName.toLowerCase(), DbCacheKey.DbInfoType.TABLE_NAMES), new Callable<ImmutableList<String>>() { @Override public ImmutableList<String> call() throws Exception { @@ -678,8 +678,8 @@ public class CatalogdMetaProvider implements MetaProvider { @Override public Pair<Table, TableMetaRef> loadTable(final String dbName, final String tableName) throws NoSuchObjectException, MetaException, TException { - // TODO(todd) need to lower case? - TableCacheKey cacheKey = new TableCacheKey(dbName, tableName); + TableCacheKey cacheKey = new TableCacheKey(dbName.toLowerCase(), + tableName.toLowerCase()); TableMetaRefImpl ref = loadWithCaching( "table metadata for " + dbName + "." + tableName, TABLE_METADATA_CACHE_CATEGORY, @@ -1280,9 +1280,8 @@ public class CatalogdMetaProvider implements MetaProvider { */ private void invalidateCacheForDb(String dbName, Iterable<DbCacheKey.DbInfoType> types, List<String> invalidated) { - // TODO(todd) check whether we need to lower-case/canonicalize dbName? for (DbCacheKey.DbInfoType type: types) { - DbCacheKey key = new DbCacheKey(dbName, type); + DbCacheKey key = new DbCacheKey(dbName.toLowerCase(), type); if (cache_.asMap().remove(key) != null) { invalidated.add(type + " for DB " + dbName); } @@ -1295,8 +1294,7 @@ public class CatalogdMetaProvider implements MetaProvider { */ private void invalidateCacheForTable(String dbName, String tblName, List<String> invalidated) { - // TODO(todd) check whether we need to lower-case/canonicalize dbName and tblName? - TableCacheKey key = new TableCacheKey(dbName, tblName); + TableCacheKey key = new TableCacheKey(dbName.toLowerCase(), tblName.toLowerCase()); if (cache_.asMap().remove(key) != null) { invalidated.add("table " + dbName + "." + tblName); } diff --git a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java index cb6c5a3..17b84ae 100644 --- a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java +++ b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java @@ -44,6 +44,8 @@ import org.apache.impala.thrift.TBackendGflags; import org.apache.impala.thrift.TCatalogObject; import org.apache.impala.thrift.TCatalogObjectType; import org.apache.impala.thrift.TDatabase; +import org.apache.impala.thrift.TFunction; +import org.apache.impala.thrift.TFunctionName; import org.apache.impala.thrift.TNetworkAddress; import org.apache.impala.thrift.TRuntimeProfileNode; import org.apache.impala.thrift.TTable; @@ -333,4 +335,61 @@ public class CatalogdMetaProviderTest { } } + // Test loading and invalidation of databases, tables with upper case + // names. Expected behavior is the local catalog should treat these + // names as case-insensitive. + @Test + public void testInvalidateObjectsCaseInsensitive() throws Exception { + provider_.loadDb("tpch"); + provider_.loadTable("tpch", "nation"); + + testInvalidateDb("TPCH"); + testInvalidateTable("TPCH", "nation"); + testInvalidateTable("tpch", "NATION"); + } + + private void testInvalidateTable(String dbName, String tblName) throws Exception { + CacheStats stats = diffStats(); + + provider_.loadTable(dbName, tblName); + + // should get a cache hit since dbName,tblName should be treated as case-insensitive + stats = diffStats(); + assertEquals(1, stats.hitCount()); + assertEquals(0, stats.missCount()); + + // Invalidate it. + TCatalogObject obj = new TCatalogObject(TCatalogObjectType.TABLE, 0); + obj.setTable(new TTable(dbName, tblName)); + provider_.invalidateCacheForObject(obj); + + // should get a cache miss if we reload it + provider_.loadTable(dbName, tblName); + stats = diffStats(); + assertEquals(0, stats.hitCount()); + assertEquals(1, stats.missCount()); + } + + private void testInvalidateDb(String dbName) throws Exception { + CacheStats stats = diffStats(); + + provider_.loadDb(dbName); + + // should get a cache hit since dbName should be treated as case-insensitive + stats = diffStats(); + assertEquals(1, stats.hitCount()); + assertEquals(0, stats.missCount()); + + // Invalidate it. + TCatalogObject obj = new TCatalogObject(TCatalogObjectType.DATABASE, 0); + obj.setDb(new TDatabase(dbName)); + provider_.invalidateCacheForObject(obj); + + // should get a cache miss if we reload it + provider_.loadDb(dbName); + stats = diffStats(); + assertEquals(0, stats.hitCount()); + assertEquals(1, stats.missCount()); + } + }
