Workflow [PR], commit [f05a714]

Summary: ✅

• Performance Comparison: Performance dashboard

AI Review

Summary

This PR adds experimental dphyp join-order enumeration plus a deterministic searched-plan budget and fallback chaining. The implementation is much closer after the latest fixes, but I still see unresolved planner completeness/costing issues and two focused test gaps around the new budget/fallback behavior.

Findings

⚠️ Majors

• [src/Processors/QueryPlan/Optimizations/joinOrder.cpp:1115] Overlapping operand-source hyperedges can still hide a valid no-cross-product plan. For a predicate such as c.x + b.y = b.z + a.w, buildHyperedges records overlapping sides {b,c} and {a,b}. After {b,c} is built, emitCsg({b,c}) excludes lower-index a through B_min, while getNeighborhood({a}) cannot expose {b,c} because neither operand side is a subset of {a}. dphyp can therefore fail/fall back even though the predicate is applicable to {b,c} JOIN {a}. [dismissed by author -- https://github.com/DPhyp join reordering algorithm for inner joins #98798#discussion_r3323752395] I still consider this real because the current getNeighborhood subset checks and B_min exclusion still follow that trace.
• [src/Processors/QueryPlan/Optimizations/joinOrder.cpp:945] The transitive-selectivity cost can still diverge from the executed join predicate set when the join already has another predicate. computeSelectivity prices the pair with a transitive equality, but cleanupJoinPredicates only synthesizes missing transitive predicates when expressions.empty(), so a join with A.v < C.v plus inferred A.k = C.k can be costed as filtered by A.k = C.k while executing without it. [dismissed by author -- https://github.com/DPhyp join reordering algorithm for inner joins #98798#discussion_r3363967825] I agree this is plan quality rather than wrong results, but the cost/execution mismatch is still present in the current code.
• [src/Processors/QueryPlan/Optimizations/joinOrder.cpp:945] The per-predicate selectivity cache is still keyed only by JoinActionRef. If a complex equality is first evaluated for a partition where a composite operand's dp_table stats are missing, computeSelectivity caches 1.0; a later valid partition reuses that unselective value even after the needed composite stats exist. Avoid caching when composite operand stats are unavailable, or include operand source sets / stats availability in the key.

Tests

• ⚠️ [tests/queries/0_stateless/03960_join_order_dphyp_search_budget.sql:50] The dpsize,greedy case still returns 5 whether dpsize honors the budget and falls back or ignores the budget and succeeds itself. Add a dpsize-only low-budget case expecting EXPERIMENTAL_FEATURE_ERROR, then keep the fallback case.
• ⚠️ [tests/queries/0_stateless/03960_join_order_dphyp_single_table_predicates.sql:9] This file still does not pin query_plan_merge_filter_into_join_condition = 0. With randomized settings it can become 1, causing the dphyp-only cases to take the merged-ON unsupported path instead of testing filter-step predicates.

Final Verdict

Status: ⚠️ Request changes

Minimum actions: fix or explicitly gate the remaining dphyp hyperedge completeness issue, address the selectivity-cache mismatch for complex operands, and strengthen the two focused tests above. The transitive-selectivity mismatch should either be fixed here or intentionally split out with the risk accepted before merge.

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered by tests: 355/383 (92.69%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 5 line(s) · Uncovered code

Full report · Diff report

@davenger performance tests are missing