adarshsanjeev commented on code in PR #16358:
URL: https://github.com/apache/druid/pull/16358#discussion_r1587482438


##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestBase.java:
##########
@@ -807,12 +808,34 @@ private MSQErrorReport getErrorReportOrThrow(String 
controllerTaskId)
 
   private void assertMSQSpec(MSQSpec expectedMSQSpec, MSQSpec querySpecForTask)
   {
-    Assert.assertEquals(expectedMSQSpec.getQuery(), 
querySpecForTask.getQuery());
+    assertMSQSpecQuery(expectedMSQSpec.getQuery(), 
querySpecForTask.getQuery());
     Assert.assertEquals(expectedMSQSpec.getAssignmentStrategy(), 
querySpecForTask.getAssignmentStrategy());
     Assert.assertEquals(expectedMSQSpec.getColumnMappings(), 
querySpecForTask.getColumnMappings());
     Assert.assertEquals(expectedMSQSpec.getDestination(), 
querySpecForTask.getDestination());
   }
 
+  private void assertMSQSpecQuery(Query msqSpecQuery, Query taskSpecQuery)

Review Comment:
   Please add some tests which check if given an sql query, the list of lookups 
are correctly populated.



##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/SqlResourceCollectorShuttle.java:
##########
@@ -87,13 +88,21 @@ public SqlNode visit(SqlIdentifier id)
         if (qualifiedNameParts.size() == 2) {
           final String schema = qualifiedNameParts.get(0);
           final String resourceName = qualifiedNameParts.get(1);
+
+          // Put the lookup names in the query context to facilitate selective 
loading of lookups.
+          if (schema.equals(NamedLookupSchema.NAME)) {
+            
plannerContext.queryContextMap().putIfAbsent(PlannerContext.CTX_LOOKUPS_TO_LOAD,
 new HashSet<>());

Review Comment:
   Since we are never initializing this value, would that allow a user to set 
CTX_LOOKUPS_TO_LOAD, and those would get added to the list and loaded?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to