weixiangsun commented on a change in pull request #8029:
URL: https://github.com/apache/pinot/pull/8029#discussion_r787189880



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
##########
@@ -85,6 +85,7 @@
   // Keep the BrokerRequest to make incremental changes
   // TODO: Remove it once the whole query engine is using the QueryContext
   private final BrokerRequest _brokerRequest;
+  private QueryContext _preAggregateGapFillQueryContext;

Review comment:
       @siddharthteotia @Jackie-Jiang 
   Q: If we are really touching the FROM clause, my suggestion would be to make 
sure we understand calcite's treatment of simple FROM clause (table name as 
today) and complex FROM clause (sub-queries).
   A: From calcite's perspective, the simple FROM clause (table name only) will 
be compiled as SqlIdentifier. The complex FROM clause will be compiled as 
SqlOrderBy object or SqlSelect object depending on if there is any order by.
   
   It might hold true in the future to make subquery as part of DataSource 
instead of making the particular gapfill sub-query.
   
   I lean towards generic approach since 
   1. it is consistent with the calcite
   2. even if it might impact the subquery feature, it can be fixed as part of 
the subquery feature development. 
   
   @siddharthteotia any other suggestion?




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