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


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/MSQWorkerTask.java:
##########
@@ -185,4 +189,15 @@ public int hashCode()
   {
     return Objects.hash(super.hashCode(), controllerTaskId, workerNumber, 
retryCount, worker);
   }
+
+  @Override
+  public LookupLoadingSpec getLookupLoadingSpec()
+  {
+    if (getContext().containsKey(PlannerContext.CTX_LOOKUPS_TO_LOAD)) {
+      List<String> lookupsToLoad = (List<String>) 
getContext().get(PlannerContext.CTX_LOOKUPS_TO_LOAD);
+      return lookupsToLoad.isEmpty() ? LookupLoadingSpec.NONE : 
LookupLoadingSpec.loadOnly(new HashSet<>(lookupsToLoad));

Review Comment:
   Hmm, I am kind of on-the-fence with this.
   
   __Pro:__ Having the extra parameter does have the advantage of explicitly 
specifying what loading mode we want to use.
   __Con:__ On the other hand, it has the added code and UX complexity of 
handling the other parameter and ensuring that the behaviour of the two is 
consistent with each other.
   
   I think when returning values from the method `getLookupLoadingSpec`, we 
needed a POJO because null and empty caused some ambiguity.
   
   But in the case of query/task context, since we are required to define a 
default, things become simpler.
   
   null / absent -> load ALL
   empty -> load NONE
   non-empty -> load ONLY_REQUIRED
   
   > Passing empty arrays seems very brittle to me. Some payload optimizations 
just remove the keys while passing stuff over the wire.
   
   By the way, I don't think payload optimizations would get rid of keys 
corresponding to empty collections, only null ones.



-- 
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