amrishlal commented on code in PR #9236:
URL: https://github.com/apache/pinot/pull/9236#discussion_r954253485


##########
pinot-core/src/test/java/org/apache/pinot/queries/BaseQueriesTest.java:
##########
@@ -146,14 +160,21 @@ private BrokerResponseNative getBrokerResponse(String 
query, PlanMaker planMaker
   /**
    * Run query on multiple index segments with custom plan maker.
    * <p>Use this to test the whole flow from server to broker.
-   * <p>The result should be equivalent to querying 4 identical index segments.
+   * <p>Unless explicitly override getDistinctInstances or initialize 2 
distinct index segments in test, the result
+   * should be equivalent to querying 4 identical index segments. Otherwise, 
the result will be equivalent to
+   * querying 2 distinct instances each having 2 segments.
    */
   private BrokerResponseNative getBrokerResponse(PinotQuery pinotQuery, 
PlanMaker planMaker) {
     PinotQuery serverPinotQuery = GapfillUtils.stripGapfill(pinotQuery);
     QueryContext queryContext = 
QueryContextConverterUtils.getQueryContext(pinotQuery);
     QueryContext serverQueryContext =
         serverPinotQuery == pinotQuery ? queryContext : 
QueryContextConverterUtils.getQueryContext(serverPinotQuery);
 
+    List<List<IndexSegment>> instances = getDistinctInstances();
+    if (instances.size() == 2) {
+      return getBrokerResponseDistinctInstances(pinotQuery, planMaker);
+    }
+

Review Comment:
   @jasperjiaguo Yes, the code can run on multiple segments, but we are 
duplicating segments in many test cases and we also duplicate server in 
BaseQueriesTest.java to mimic realtime server. I think @SabrinaZhaozyf is 
adding the case for running queries with two servers on two segments (?). At 
some point, I guess, this file could be cleaned up a bit to setup all the 
common cases more cleanly: 1) single server single segment, 2) single server 
multiple segments, 3) two servers, one segement each, etc... But that is 
probably another PR.
   
   I am ok with the if check ( `if (instances.size() == 2)`). I confused this a 
bit with other cases.



-- 
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-unsubscr...@pinot.apache.org

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


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

Reply via email to