From 3eb2bb8f4ada865f3cbaabd117115c1542e31eac Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 18 Apr 2024 12:39:00 +0530 Subject: [PATCH 1/3] Fix NPE while loading lookups from empty JDBC source --- .../lookup/namespace/JdbcCacheGenerator.java | 22 +++++++------- .../NamespacedExtractorModuleTest.java | 2 +- .../cache/JdbcExtractionNamespaceTest.java | 30 +++++++++++++++++++ 3 files changed, 43 insertions(+), 11 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 f05ff702504d..d8b202d0d53e 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 @@ -204,18 +204,20 @@ private Long lastUpdates(CacheScheduler.EntryImpl key, if (tsColumn == null) { return null; } + final String query = StringUtils.format( + "SELECT MAX(%s) FROM %s", + tsColumn, table + ); final Timestamp update = dbi.withHandle( - handle -> { - final String query = StringUtils.format( - "SELECT MAX(%s) FROM %s", - tsColumn, table - ); - return handle - .createQuery(query) - .map(TimestampMapper.FIRST) - .first(); - } + handle -> handle + .createQuery(query) + .map(TimestampMapper.FIRST) + .first() ); + if (update == null){ + LOG.info("Lookup table is empty. No rows returned for the query: %s", query); + return null; + } return update.getTime(); } } diff --git a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/NamespacedExtractorModuleTest.java b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/NamespacedExtractorModuleTest.java index 715da7bed924..4e04f034cab6 100644 --- a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/NamespacedExtractorModuleTest.java +++ b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/NamespacedExtractorModuleTest.java @@ -121,7 +121,7 @@ public void testNewTask() throws Exception Assert.assertNotNull(version); Map map = cache.getCache(); Assert.assertEquals("bar", map.get("foo")); - Assert.assertEquals(null, map.get("baz")); + Assert.assertNull(map.get("baz")); } @Test 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 cf71a5da4902..ada8e2ee9264 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 @@ -390,6 +390,36 @@ public void testMappingWithFilter() } } + @Test(timeout = 60_000L) + public void testEmptyTable() + throws InterruptedException + { + // Delete existing rows from table. + final Handle handle = derbyConnectorRule.getConnector().getDBI().open(); + handle.createStatement( + StringUtils.format("DELETE FROM %s", TABLE_NAME) + ).setQueryTimeout(1).execute(); + + final JdbcExtractionNamespace extractionNamespace = new JdbcExtractionNamespace( + derbyConnectorRule.getMetadataConnectorConfig(), + TABLE_NAME, + KEY_NAME, + VAL_NAME, + tsColumn, + null, + new Period(0), + null, + 0, + null, + new JdbcAccessSecurityConfig() + ); + try (CacheScheduler.Entry entry = scheduler.schedule(extractionNamespace)) { + CacheSchedulerTest.waitFor(entry); + final Map map = entry.getCache(); + Assert.assertTrue(map.isEmpty()); + } + } + @Test(timeout = 60_000L) public void testSkipOld() throws InterruptedException From b68cdcaf960305350b553774d84fc6583302ce7b Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 18 Apr 2024 13:44:39 +0530 Subject: [PATCH 2/3] Fix formatting --- .../druid/server/lookup/namespace/JdbcCacheGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d8b202d0d53e..b860c8b19fdf 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 @@ -214,7 +214,7 @@ private Long lastUpdates(CacheScheduler.EntryImpl key, .map(TimestampMapper.FIRST) .first() ); - if (update == null){ + if (update == null) { LOG.info("Lookup table is empty. No rows returned for the query: %s", query); return null; } From e3604a2222f2b63595f8cca3c101950ddd38fb35 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 18 Apr 2024 19:09:38 +0530 Subject: [PATCH 3/3] Address review comment --- .../druid/server/lookup/namespace/JdbcCacheGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b860c8b19fdf..ccf8504875d1 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 @@ -215,7 +215,7 @@ private Long lastUpdates(CacheScheduler.EntryImpl key, .first() ); if (update == null) { - LOG.info("Lookup table is empty. No rows returned for the query: %s", query); + LOG.info("Lookup table[%s] is empty. No rows returned for the query[%s].", table, query); return null; } return update.getTime();