github-actions[bot] commented on PR #61842:
URL: https://github.com/apache/doris/pull/61842#issuecomment-4148431792

   ## Code Review Summary
   
   **Scope**: This PR consolidates unique test content from `nereids_p0` 
regression tests into `query_p0`, eliminating duplication now that the Nereids 
optimizer is the default. ~1201 files changed (34k additions, 88k deletions), 
almost entirely test file reorganization.
   
   ### Reviewed Checkpoints
   
   1. **Obsolete settings removed** — Confirmed: zero new additions of 
`enable_nereids_planner`, `enable_fallback_to_original_planner`, or 
`nereids_test_query_db` in any added lines. All legacy Nereids-specific 
settings have been properly cleaned up.
   
   2. **Test patterns** — Tests correctly use `qt_`, `order_qt_`, `test { }`, 
and `explain { }` patterns. Tables are dropped with `IF EXISTS` before creation 
(34 instances verified).
   
   3. **Session variable management** — Properly handled. For example in 
`test_subquery.groovy`, `disable_nereids_rules` is set and reset correctly 
around relevant test blocks.
   
   4. **`.out` golden files** — Updated consistently to match new `qt_` test 
case additions and path renames from `nereids_p0` to `query_p0`.
   
   5. **No newline-at-EOF issues** in newly added content.
   
   ### Minor Issue
   
   In 
`regression-test/suites/query_p0/aggregate/aggregate_group_by_metric_type.groovy`,
 there is a `DROP TABLE test_group_by_array` at the end of the test without `IF 
EXISTS`. Per test standards, tables should be dropped *before* use (not after), 
and should use `IF EXISTS`. This is a pre-existing issue carried over from 
`nereids_p0`, so it's minor and non-blocking.
   
   ### Verdict
   
   The PR is a straightforward and well-executed test consolidation. No 
functional concerns found.


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

Reply via email to