siddharthteotia commented on a change in pull request #8029:
URL: https://github.com/apache/pinot/pull/8029#discussion_r787173187
##########
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:
@Jackie-Jiang
So this was one of the things which was discussed a lot. My concern was that
changing the PinotQuery now to accommodate "**generic**" subquery has to be
done carefully accounting for standard sql subquery syntax and semantics and we
should be confident that it will hold in future
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). Whatever we do today in the FROM
clause to make this particular gapfill sub-query work should not interfere in
the future when we are going to leverage / extend calcite's generic subquery
planner to support all kinds of subqueries.
So this is why the path of least resistance could be to not touch PinotQuery
for generic Subquery support since that may require a lot of design thinking
right away before agreeing upon how to model any subquery in Pinot and block
the current feature. So may be making it specific like in the above code just
for gapfill is the way to go ?
--
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]
