You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/10/26 23:06:55 UTC

[GitHub] [pinot] walterddr commented on a diff in pull request #9653: [multistage] visitor pattern for server request construction

walterddr commented on code in PR #9653:
URL: https://github.com/apache/pinot/pull/9653#discussion_r1006260404


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/QueryRunner.java:
##########
@@ -112,13 +120,14 @@ public void processQuery(DistributedStagePlan distributedStagePlan, ExecutorServ
       // TODO: make server query request return via mailbox, this is a hack to gather the non-streaming data table
       // and package it here for return. But we should really use a MailboxSendOperator directly put into the
       // server executor.
-      List<ServerQueryRequest> serverQueryRequests =
-          ServerRequestUtils.constructServerQueryRequest(distributedStagePlan, requestMetadataMap,
-              _helixPropertyStore);
+      List<ServerPlanRequestContext> serverQueryRequests = constructServerQueryRequests(distributedStagePlan,
+          requestMetadataMap, _helixPropertyStore);

Review Comment:
   The goal here is for the future 
   
   1. by returning the `ServerPlanRequestContext`, we will be able to process the compiled pinotQuery using v1 engine; but also potentially process other part of the plan like dynamic filtering or anything that depend on another stage in leaf node
   2. able to later directly generate the operator chain instead of having to run another planMaker step.
   3. 



##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/QueryRunner.java:
##########
@@ -112,13 +120,14 @@ public void processQuery(DistributedStagePlan distributedStagePlan, ExecutorServ
       // TODO: make server query request return via mailbox, this is a hack to gather the non-streaming data table
       // and package it here for return. But we should really use a MailboxSendOperator directly put into the
       // server executor.
-      List<ServerQueryRequest> serverQueryRequests =
-          ServerRequestUtils.constructServerQueryRequest(distributedStagePlan, requestMetadataMap,
-              _helixPropertyStore);
+      List<ServerPlanRequestContext> serverQueryRequests = constructServerQueryRequests(distributedStagePlan,
+          requestMetadataMap, _helixPropertyStore);

Review Comment:
   The goal here is for the future 
   
   1. by returning the `ServerPlanRequestContext`, we will be able to process the compiled pinotQuery using v1 engine; but also potentially process other part of the plan like dynamic filtering or anything that depend on another stage in leaf node
   2. able to later directly generate the operator chain instead of having to run another planMaker step.
   



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org