manakbisht commented on code in PR #20148:
URL: https://github.com/apache/pinot/pull/20148#discussion_r4479749924
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -745,6 +746,7 @@ protected BrokerResponse doHandleRequest(long requestId,
String query, SqlNodeAn
setTableName(serverBrokerRequest, realtimeTableName);
handleExpressionOverride(serverPinotQuery,
_tableCache.getExpressionOverrideMap(realtimeTableName));
handleTimestampIndexOverride(serverPinotQuery, realtimeTableConfig);
+ _queryOptimizer.optimize(serverPinotQuery, schema);
Review Comment:
Thanks for flagging this. I checked it, and the SPLIT path isn't affected
for OFFLINE-only/REALTIME-only base tables. In those branches,
`doHandleRequest` applies the override to `serverPinotQuery` in place
(L738/L751) and, with this PR, re-optimizes it. That same object is what
`tryExecuteMaterializedViewSplit` passes as `baseServerPinotQuery`.
`DefaultMaterializedViewHandler.executeSplit` deep-copies it, and
`dispatchMaterializedViewSplit` optimizes it again before
`prepareBaseTableHybridRoute`. So by the time `prepareBaseTableHybridRoute`
calls `handleExpressionOverride`, there's nothing left to replace. For hybrid
tables, the main path overrides deep copies, and that branch of
`prepareBaseTableHybridRoute` already re-optimizes.
I also checked it end to end on a local cluster: an OFFLINE base table with
the time pruner and `REGEXP_LIKE(an,'^.*') -> TRUE`, a materialized view over
it, and a SPLIT query (`... WHERE pt = 'x' AND REGEXP_LIKE(an, '^.*') GROUP BY
pt`). With only this PR's change, the query is served through the view
(`materializedViewQueried` is set), returns the correct result, and the broker
logs no split failure or fallback. On master it fails with the NPE in the main
routing path. So I'd rather not add the extra calls in
`prepareBaseTableHybridRoute`, since they'd have no effect.
--
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]
