gortiz commented on PR #20061: URL: https://github.com/apache/pinot/pull/20061#issuecomment-6502783052
Thanks, this is a much better default than off. I took the prototype and made `auto` the default mode in e4fc71fb4f. The gate now has three values, `auto`, `true` and `false`, in the broker config, the cluster config and the `useMvAggregate` query option. In `auto`, `CompiledQuery` plans the query as master does. Only when `ArrayToMvValidationVisitor` throws the new `ArrayToMvValidationException` does it optimize the kept `RelRoot` again with the MV aggregate on. An explicit `true` or `false` disables the fallback, and `SET useMvAggregate='auto'` turns it on for one query when the broker default is `false`. What I added on top of the prototype: - `EXPLAIN PLAN WITHOUT IMPLEMENTATION` in `auto` converts the plan into fragments and runs the validation on them, without worker assignment. It does this only when the plan contains `ARRAY_TO_MV`. So it shows the plan that the query runs. - The `UNNEST` leaf boundary from your prototype is kept, with a plan test and a runtime case (`CROSS JOIN UNNEST` plus `GROUP BY ARRAY_TO_MV` now works in `auto`). - `PinotRuleUtils.isMvAggregateEnabled` fails closed without a context (also Xiang's note). - The `HAVING` fix and the broker restart fix, see the inline replies. The test base now uses the default, so `ResourceBasedQueryPlansTest` and `ResourceBasedQueriesTest` run in `auto`. Cases with an explicit `SET useMvAggregate` keep their mode. -- 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]
