agavra commented on code in PR #9895:
URL: https://github.com/apache/pinot/pull/9895#discussion_r1040180019


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/HashJoinOperator.java:
##########
@@ -114,14 +125,98 @@ protected TransferableBlock getNextBlock() {
         return TransferableBlockUtils.getNoOpTransferableBlock();
       }
       // JOIN each left block with the right block.
-      return buildJoinedDataBlock(_leftTableOperator.nextBlock());
+      return buildJoinedDataBlock(getProbeOperator().nextBlock());
     } catch (Exception e) {
       return TransferableBlockUtils.getErrorTransferableBlock(e);
     }
   }
 
+  private Operator<TransferableBlock> getBroadcastOperator() {

Review Comment:
   nit (suggestion): might be nicer to wrap all of these in a single Tuple 
class so that we can have only once switch statement:
   ```
   static class JoinResolver {
     Operator<TransferableBlock> broadcastOperator;
     Operator<TransferableBlock> probeOperator;
     KeySelector broadcastSelector;
     KeySelector probOperator;
     BiFunction<Object[], Object[]> getLeftRow;
     BiFunction<Object[], Object[]> getRightRow;
   }
   ```
   
   and then just:
   ```
   switch(_joinType) {
     case Left:
       return new JoinResolver(...)
   }
   ```



##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -22,7 +22,7 @@
  * Configuration for setting up query runtime.
  */
 public class QueryConfig {
-  public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000L;
+  public static final long DEFAULT_TIMEOUT_NANO = 10_000_000_000_000L;

Review Comment:
   (MAJOR) let's not forget to revert this!



##########
pinot-query-runtime/src/test/resources/queries/FromExpressions.json:
##########
@@ -57,9 +57,14 @@
       },
       {
         "psql": "7.2.1.1",
-        "ignored": true,
+        "comments": "select all doesn't work because h2 output the rows in 
different order from Pinot and test framework doesn't consider the column order 
for rows and blindly assume they are in the same order",

Review Comment:
   can we add a test for this with the outputs specified?



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