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, I think, for 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 payload optimizations would get rid of 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]
