Skip to content

[fix](regression) stabilize DPHyp query85 shape check#65461

Draft
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:agent/stabilize-dphyper-query85-time-limit
Draft

[fix](regression) stabilize DPHyp query85 shape check#65461
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:agent/stabilize-dphyper-query85-time-limit

Conversation

@shuke987

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: None

Related PR: #64559

Problem Summary:

The TPC-DS SF1000 DPHyp query85 case checks an exact physical plan shape. It already pins the enumeration-count budget with dphyper_limit=2600, but it leaves the independent wall-clock join_reorder_time_limit at its 1000 ms default.

PlanReceiver stops full enumeration when either limit is exceeded. When that happens, JoinOrderJob invokes GraphSimplifier and re-enumerates a reduced candidate space, which can select a different valid join order and fail the exact shape golden.

The TeamCity timing split matches that boundary:

Build Shape result Time from Execute tag to generated output First differing join
991071 PASS 864 ms expected web_page join
989238 FAILURE 1378 ms customer_address join
989267 FAILURE 1351 ms cd1 join
989379 FAILURE 1444 ms customer_address join
991088 FAILURE 1361 ms date_dim join

The four genuine join-order failures all crossed the approximately one-second boundary. The current failing PR #65434 changes only file-cache code, so the failure is not caused by its product diff. Separate qualifier-only failures from PRs #65346 and #65264 are not part of this diagnosis.

This PR pins join_reorder_time_limit=10000 for query85 only. It keeps dphyper_limit=2600, the SQL, and the expected golden unchanged, so the test continues to validate the intended DPHyp plan while avoiding load-dependent fallback.

Validation:

  • git diff --check
  • Verified the patch changes only the query85 session setup.
  • Local single-suite execution was not run because no Doris regression environment was allocated for this task.
  • run buildall and the exact P0 case will provide CI validation on the patched head.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@shuke987

Copy link
Copy Markdown
Collaborator Author

run buildall

@shuke987

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jul 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review result: requesting changes for one regression-test failure.

Critical checkpoint conclusions:

  • Goal/test proof: the PR tries to stabilize the DPHyp TPC-DS Q85 shape check by raising the join-order timeout, but the added SET uses an unregistered variable name, so the suite fails before the shape check proves anything.
  • Scope: the change is small and focused, but the typo prevents the intended behavior.
  • Concurrency, lifecycle, transactions, data writes, persistence, FE-BE protocol, observability, and compatibility: not involved beyond ordinary session-variable handling.
  • Parallel paths: both DPHyp PlanReceiver implementations consume SessionVariable.joinReorderTimeLimit; the issue is that the test does not set the registered variable name.
  • Tests/results: the expected output file is unchanged, and the regression cannot reach it until the SET line is corrected. I did not run the shape suite in this runner; validation was static against the PR patch, SessionVariable, and VariableMgr.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions: tests-session-config proposed TEST-1, accepted as MAIN-1 and submitted inline. optimizer-rewrite proposed OPT-1 as a duplicate of TEST-1, merged into MAIN-1. Convergence round 1 ended with both live subagents returning NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set.

sql "SET enable_dphyp_optimizer = true"
sql "set max_table_count_use_cascades_join_reorder = 1"
sql "set dphyper_limit=2600"
sql "set join_reorder_time_limit=10000"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setting name does not exist in Doris. The registered session variable is join_order_time_limit (SessionVariable.JOIN_REORDER_TIME_LIMIT), and joinReorderTimeLimit is annotated with that name; for an unknown name, VariableMgr.setVar raises ERR_UNKNOWN_SYSTEM_VARIABLE. As written, this test fails on the SET before it reaches the qt_ds_shape_85 plan check, so it will not stabilize the DPHyp shape. Did you mean set join_order_time_limit=10000?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants