From f5b4fa41a9cd0bdc6bbacb1b56b58b058f6b9171 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Wed, 5 Jun 2024 22:19:50 +0530 Subject: [PATCH 1/4] Add interface method for returning canonical lookup name --- .../org/apache/druid/benchmark/JoinAndLookupBenchmark.java | 6 ++++++ .../lookup/LookupExtractorFactoryContainerProvider.java | 5 +++++ .../apache/druid/query/expression/LookupExprMacroTest.java | 6 ++++++ .../apache/druid/query/lookup/LookupReferencesManager.java | 6 ++++++ .../org/apache/druid/query/lookup/LookupSerdeModule.java | 6 ++++++ .../query/expression/LookupEnabledTestExprMacroTable.java | 6 ++++++ .../org/apache/druid/segment/LookupSegmentWranglerTest.java | 6 ++++++ .../druid/segment/join/LookupJoinableFactoryTest.java | 6 ++++++ .../druid/server/SpecificSegmentsQuerySegmentWalker.java | 6 ++++++ .../druid/server/AsyncQueryForwardingServletTest.java | 6 ++++++ .../expression/builtin/QueryLookupOperatorConversion.java | 2 +- .../apache/druid/sql/calcite/util/TestLookupProvider.java | 6 ++++++ 12 files changed, 66 insertions(+), 1 deletion(-) diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java index 7ac9931da48f..ba07091fe5ae 100644 --- a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java @@ -305,6 +305,12 @@ public Optional get(String lookupName) return Optional.empty(); } } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } ) ) diff --git a/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java b/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java index 1a61520903b2..4c18eec0dd8c 100644 --- a/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java +++ b/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java @@ -40,4 +40,9 @@ public interface LookupExtractorFactoryContainerProvider * Returns a lookup container for the provided lookupName, if it exists. */ Optional get(String lookupName); + + /** + * Returns the canonical lookup name from a lookup name. + */ + String getCanonicalLookupName(String lookupName); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java index 4db019210461..cef936729e7c 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/LookupExprMacroTest.java @@ -53,6 +53,12 @@ public Optional get(String lookupName) { return Optional.empty(); } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } }) ); } diff --git a/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java b/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java index 32b7589dbba0..23e6ec48b8f4 100644 --- a/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java +++ b/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java @@ -332,6 +332,12 @@ public Set getAllLookupNames() return stateRef.get().lookupMap.keySet(); } + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } + // Note that this should ensure that "toLoad" and "toDrop" are disjoint. LookupsState getAllLookupsState() { diff --git a/server/src/main/java/org/apache/druid/query/lookup/LookupSerdeModule.java b/server/src/main/java/org/apache/druid/query/lookup/LookupSerdeModule.java index d8a981b8db46..e0d9278e4b15 100644 --- a/server/src/main/java/org/apache/druid/query/lookup/LookupSerdeModule.java +++ b/server/src/main/java/org/apache/druid/query/lookup/LookupSerdeModule.java @@ -79,5 +79,11 @@ public Optional get(String lookupName) { return Optional.empty(); } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } } diff --git a/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java b/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java index fe40b9c5b732..24885e2daf58 100644 --- a/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java +++ b/server/src/test/java/org/apache/druid/query/expression/LookupEnabledTestExprMacroTable.java @@ -95,6 +95,12 @@ public Optional get(String lookupName) return Optional.empty(); } } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } }; } diff --git a/server/src/test/java/org/apache/druid/segment/LookupSegmentWranglerTest.java b/server/src/test/java/org/apache/druid/segment/LookupSegmentWranglerTest.java index 228b17c4b67e..9d8e2653cefb 100644 --- a/server/src/test/java/org/apache/druid/segment/LookupSegmentWranglerTest.java +++ b/server/src/test/java/org/apache/druid/segment/LookupSegmentWranglerTest.java @@ -67,6 +67,12 @@ public Optional get(final String lookupName) return Optional.empty(); } } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } ); diff --git a/server/src/test/java/org/apache/druid/segment/join/LookupJoinableFactoryTest.java b/server/src/test/java/org/apache/druid/segment/join/LookupJoinableFactoryTest.java index 6e0e737db9ed..160cf74f9655 100644 --- a/server/src/test/java/org/apache/druid/segment/join/LookupJoinableFactoryTest.java +++ b/server/src/test/java/org/apache/druid/segment/join/LookupJoinableFactoryTest.java @@ -79,6 +79,12 @@ public Optional get(String lookupName) return Optional.empty(); } } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } ); } diff --git a/server/src/test/java/org/apache/druid/server/SpecificSegmentsQuerySegmentWalker.java b/server/src/test/java/org/apache/druid/server/SpecificSegmentsQuerySegmentWalker.java index 4f70f01cf54d..d9301f28cc00 100644 --- a/server/src/test/java/org/apache/druid/server/SpecificSegmentsQuerySegmentWalker.java +++ b/server/src/test/java/org/apache/druid/server/SpecificSegmentsQuerySegmentWalker.java @@ -89,6 +89,12 @@ public Optional get(String lookupName) { return Optional.empty(); } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } }; public static SpecificSegmentsQuerySegmentWalker createWalker( diff --git a/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java b/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java index 29831f3c4302..11d734bec291 100644 --- a/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java +++ b/services/src/test/java/org/apache/druid/server/AsyncQueryForwardingServletTest.java @@ -481,6 +481,12 @@ public Optional get(String lookupName) { return Optional.empty(); } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } final TimeseriesQuery query = diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/QueryLookupOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/QueryLookupOperatorConversion.java index 8947bd60a01f..af4ecd9426d6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/QueryLookupOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/QueryLookupOperatorConversion.java @@ -84,7 +84,7 @@ public DruidExpression toDruidExpression( final String lookupName = (String) lookupNameExpr.getLiteralValue(); // Add the lookup name to the set of lookups to selectively load. - plannerContext.addLookupToLoad(lookupName); + plannerContext.addLookupToLoad(lookupExtractorFactoryContainerProvider.getCanonicalLookupName(lookupName)); if (arg.isSimpleExtraction() && lookupNameExpr.isLiteral()) { return arg.getSimpleExtraction().cascade( diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/TestLookupProvider.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/TestLookupProvider.java index f4f8bd14560d..2fbfad5d82e2 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/util/TestLookupProvider.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/TestLookupProvider.java @@ -55,4 +55,10 @@ public Optional get(String lookupName) return Optional.empty(); } } + + @Override + public String getCanonicalLookupName(String lookupName) + { + return lookupName; + } } From ee9d0e023126eb1c3c321397bde48a4dc9a7f3e7 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Wed, 5 Jun 2024 22:59:39 +0530 Subject: [PATCH 2/4] Address review comment --- .../query/lookup/LookupExtractorFactoryContainerProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java b/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java index 4c18eec0dd8c..35036ec9dbcd 100644 --- a/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java +++ b/processing/src/main/java/org/apache/druid/query/lookup/LookupExtractorFactoryContainerProvider.java @@ -42,7 +42,7 @@ public interface LookupExtractorFactoryContainerProvider Optional get(String lookupName); /** - * Returns the canonical lookup name from a lookup name. + * Returns the canonical lookup name from a given lookup name, if special syntax exists. */ String getCanonicalLookupName(String lookupName); } From 2a255ee58f2336d13f22d41843a4780ce7ca28a3 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Wed, 5 Jun 2024 23:12:52 +0530 Subject: [PATCH 3/4] Add test in LookupReferencesManagerTest for coverage check --- .../druid/query/lookup/LookupReferencesManagerTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java b/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java index 442a2b478f59..992c82022091 100644 --- a/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java +++ b/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java @@ -579,6 +579,13 @@ public void testGetAllLookupNames() throws Exception ); } + @Test + public void testGetCanonicalLookupName() + { + String lookupName = "lookupName1"; + Assert.assertEquals(lookupName, lookupReferencesManager.getCanonicalLookupName(lookupName)); + } + @Test public void testGetAllLookupsState() throws Exception { From 42417b9b10f46712a22a013900e6007da3a73009 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Wed, 5 Jun 2024 23:18:43 +0530 Subject: [PATCH 4/4] Add test in LookupSerdeModuleTest for coverage check --- .../apache/druid/query/lookup/LookupSerdeModuleTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java b/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java index dfa98534fede..297870af373d 100644 --- a/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java +++ b/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java @@ -117,4 +117,12 @@ public void testExpressionTransformSerde() throws Exception objectMapper.readValue(objectMapper.writeValueAsBytes(transform), ExpressionTransform.class) ); } + + @Test + public void testGetCanonicalLookupName() + { + LookupExtractorFactoryContainerProvider instance = injector.getInstance(LookupExtractorFactoryContainerProvider.class); + String lookupName = "lookupName1"; + Assert.assertEquals(lookupName, instance.getCanonicalLookupName(lookupName)); + } }