github-actions[bot] commented on code in PR #65505:
URL: https://github.com/apache/doris/pull/65505#discussion_r3568416194
##########
regression-test/suites/inverted_index_p0/test_multi_match_null_query.groovy:
##########
@@ -36,8 +36,10 @@ suite("test_multi_match_null_query", "p0") {
// Disable FE constant folding so NULL is not optimized to VEMPTYSET,
// forcing the predicate to reach the BE inverted index path
(pushAggOp=COUNT_ON_INDEX).
sql "SET disable_nereids_expression_rules = 'FOLD_CONSTANT_ON_FE'"
- sql "SET enable_fold_constant_by_be = 'true'"
- sql "SET enable_sql_cache = 0"
+ sql "SET enable_fold_constant_by_be = true"
+ sql "SET enable_sql_cache = false"
+ sql "SET enable_common_expr_pushdown = true"
Review Comment:
These two SETs do not actually force the path the test comment describes.
Both `enable_common_expr_pushdown` and
`enable_common_expr_pushdown_for_inverted_index` are listed in
`VariableMgr.REMOVED_SESSION_VAR_NAMES`, so `VariableMgr.setVar()` silently
returns without changing the session. The current count-on-index rewrite is
gated by `enable_count_on_index_pushdown` via
`SessionVariable.isEnablePushDownCountOnIndex()`, and this test only checks the
result rows, not that the plan contains `pushAggOp=COUNT_ON_INDEX`. With
today's defaults it may still happen to use the intended path, but the
regression will stop proving the bug as soon as the default/session state
changes. Please set the real gate and add an `EXPLAIN` assertion for
`pushAggOp=COUNT_ON_INDEX` (or remove these dead SETs if the test is meant to
be result-only).
--
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]
