planner/core: disable new join reorder when DP is enabled by guo-shaoge · Pull Request #67079 · pingcap/tidb

guo-shaoge · 2026-03-17T05:23:14Z

What problem does this PR solve?

Issue Number: close #67073

Problem Summary:
The original join order of tpch q9 is like: ((nation supplier) (part lineitem)) partsupp orders, which is generated by DP.

After #66349, the join order became: (((nation suppolier) lineitem) part) partsupp orders, because #66349 enable the new join reorder impl by default, which only support greedy algorithm for now. And the development of DP in the new join reorder impl is still undergoing.

But in our benchbot env, DP is used explicitly by setting tidb_opt_join_reorder_threshold as 60(DP is disabled by default, you have to set this variable greater than zero then you can use DP), so we have to respect that value for now. And after DP is supported in the new join reorder impl, I'll revert this pr.

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

setup tidb cluster with 2 tiflash with tpch-50G dataset.

before this pr: (nation supplier) lineitem part

mysql> explain select     nation,     o_year,     sum(amount) as sum_profit from     (         select             n_name as nation,             extract(year from o_orderdate) as o_year,             l_extendedprice * (1 - l_discount) - ps_supplycost * l_quantity as amount         from             part,             supplier,             lineitem,             partsupp,             orders,             nation         where             s_suppkey = l_suppkey             and ps_suppkey = l_suppkey             and ps_partkey = l_partkey
      and p_partkey = l_partkey             and o_orderkey = l_orderkey             and s_nationkey = n_nationkey             and p_name like '%dim%'     ) as profit group by     nation,     o_year order by     nation,     o_year desc;
+--------------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                                                                                         | estRows      | task         | access object  | operator info                                                                                                                                                                                                                          |
+--------------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Sort_27                                                                                    | 2406.00      | root         |                | test.nation.n_name, Column#57:desc                                                                                                                                                                                                     |
| └─TableReader_344                                                                          | 2406.00      | root         |                | MppVersion: 3, data:ExchangeSender_343                                                                                                                                                                                                 |
|   └─ExchangeSender_343                                                                     | 2406.00      | mpp[tiflash] |                | ExchangeType: PassThrough                                                                                                                                                                                                              |
|     └─Projection_30                                                                        | 2406.00      | mpp[tiflash] |                | test.nation.n_name, Column#57, Column#59                                                                                                                                                                                               |
|       └─Projection_339                                                                     | 2406.00      | mpp[tiflash] |                | Column#59, test.nation.n_name, Column#57                                                                                                                                                                                               |
|         └─HashAgg_340                                                                      | 2406.00      | mpp[tiflash] |                | group by:Column#70, test.nation.n_name, funcs:sum(Column#71)->Column#59, funcs:firstrow(test.nation.n_name)->test.nation.n_name, funcs:firstrow(Column#70)->Column#57, stream_count: 72                                                |
|           └─ExchangeReceiver_342                                                           | 2406.00      | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|             └─ExchangeSender_341                                                           | 2406.00      | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.nation.n_name, collate: utf8mb4_bin], stream_count: 72                                                                                                          |
|               └─HashAgg_337                                                                | 2406.00      | mpp[tiflash] |                | group by:Column#75, Column#76, funcs:sum(Column#74)->Column#71                                                                                                                                                                         |
|                 └─Projection_347                                                           | 247796839.16 | mpp[tiflash] |                | minus(mul(test.lineitem.l_extendedprice, minus(1, test.lineitem.l_discount)), mul(test.partsupp.ps_supplycost, test.lineitem.l_quantity))->Column#74, test.nation.n_name->Column#75, extract(YEAR, test.orders.o_orderdate)->Column#76 |
|                   └─Projection_326                                                         | 247796839.16 | mpp[tiflash] |                | test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.partsupp.ps_supplycost, test.orders.o_orderdate, test.nation.n_name                                                                            |
|                     └─Projection_316                                                       | 247796839.16 | mpp[tiflash] |                | test.nation.n_name, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.partsupp.ps_supplycost, test.orders.o_orderdate, test.lineitem.l_orderkey                                                  |
|                       └─HashJoin_315                                                       | 247796839.16 | mpp[tiflash] |                | inner join, equal:[eq(test.lineitem.l_orderkey, test.orders.o_orderkey)], stream_count: 72                                                                                                                                             |
|                         ├─ExchangeReceiver_88(Build)                                       | 75000000.00  | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                         │ └─ExchangeSender_87                                              | 75000000.00  | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.orders.o_orderkey, collate: binary], stream_count: 72                                                                                                           |
|                         │   └─TableFullScan_86                                             | 75000000.00  | mpp[tiflash] | table:orders   | keep order:false                                                                                                                                                                                                                       |
|                         └─ExchangeReceiver_85(Probe)                                       | 244189657.27 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                           └─ExchangeSender_84                                              | 244189657.27 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_orderkey, collate: binary]                                                                                                                           |
|                             └─Projection_83                                                | 244189657.27 | mpp[tiflash] |                | test.nation.n_name, test.lineitem.l_orderkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.partsupp.ps_supplycost, test.lineitem.l_suppkey, test.lineitem.l_partkey                         |
|                               └─HashJoin_56                                                | 244189657.27 | mpp[tiflash] |                | inner join, equal:[eq(test.lineitem.l_suppkey, test.partsupp.ps_suppkey) eq(test.lineitem.l_partkey, test.partsupp.ps_partkey)], stream_count: 72                                                                                      |
|                                 ├─ExchangeReceiver_80(Build)                               | 40000000.00  | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                 │ └─ExchangeSender_79                                      | 40000000.00  | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.partsupp.ps_suppkey, collate: binary], [name: test.partsupp.ps_partkey, collate: binary], stream_count: 72                                                      |
|                                 │   └─TableFullScan_78                                     | 40000000.00  | mpp[tiflash] | table:partsupp | keep order:false                                                                                                                                                                                                                       |
|                                 └─ExchangeReceiver_82(Probe)                               | 241498491.89 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                   └─ExchangeSender_81                                      | 241498491.89 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_suppkey, collate: binary], [name: test.lineitem.l_partkey, collate: binary]                                                                          |
|                                     └─Projection_77                                        | 241498491.89 | mpp[tiflash] |                | test.nation.n_name, test.lineitem.l_orderkey, test.lineitem.l_partkey, test.lineitem.l_suppkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount                                                      |
|                                       └─HashJoin_57                                        | 241498491.89 | mpp[tiflash] |                | inner join, equal:[eq(test.lineitem.l_partkey, test.part.p_partkey)], stream_count: 72                                                                                                                                                 |
|                                         ├─ExchangeReceiver_76(Build)                       | 8000000.00   | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                         │ └─ExchangeSender_75                              | 8000000.00   | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.part.p_partkey, collate: binary], stream_count: 72                                                                                                              |
|                                         │   └─Selection_74                                 | 8000000.00   | mpp[tiflash] |                | like(test.part.p_name, "%dim%", 92)                                                                                                                                                                                                    |
|                                         │     └─TableFullScan_73                           | 10000000.00  | mpp[tiflash] | table:part     | keep order:false                                                                                                                                                                                                                       |
|                                         └─ExchangeReceiver_72(Probe)                       | 300833705.36 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                           └─ExchangeSender_71                              | 300833705.36 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_partkey, collate: binary]                                                                                                                            |
|                                             └─Projection_70                                | 300833705.36 | mpp[tiflash] |                | test.nation.n_name, test.lineitem.l_orderkey, test.lineitem.l_partkey, test.lineitem.l_suppkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount                                                      |
|                                               └─HashJoin_58                                | 300833705.36 | mpp[tiflash] |                | inner join, equal:[eq(test.supplier.s_suppkey, test.lineitem.l_suppkey)], stream_count: 72                                                                                                                                             |
|                                                 ├─ExchangeReceiver_66(Build)               | 500000.00    | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                                 │ └─ExchangeSender_65                      | 500000.00    | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.supplier.s_suppkey, collate: binary], stream_count: 72                                                                                                          |
|                                                 │   └─Projection_64                        | 500000.00    | mpp[tiflash] |                | test.nation.n_name, test.supplier.s_suppkey                                                                                                                                                                                            |
|                                                 │     └─HashJoin_59                        | 500000.00    | mpp[tiflash] |                | inner join, equal:[eq(test.nation.n_nationkey, test.supplier.s_nationkey)]                                                                                                                                                             |
|                                                 │       ├─ExchangeReceiver_62(Build)       | 25.00        | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                                 │       │ └─ExchangeSender_61              | 25.00        | mpp[tiflash] |                | ExchangeType: Broadcast, Compression: FAST                                                                                                                                                                                             |
|                                                 │       │   └─TableFullScan_60             | 25.00        | mpp[tiflash] | table:nation   | keep order:false                                                                                                                                                                                                                       |
|                                                 │       └─TableFullScan_63(Probe)          | 500000.00    | mpp[tiflash] | table:supplier | keep order:false                                                                                                                                                                                                                       |
|                                                 └─ExchangeReceiver_69(Probe)               | 300005811.00 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                                   └─ExchangeSender_68                      | 300005811.00 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_suppkey, collate: binary]                                                                                                                            |
|                                                     └─TableFullScan_67                     | 300005811.00 | mpp[tiflash] | table:lineitem | keep order:false                                                                                                                                                                                                                       |
+--------------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
46 rows in set (0.01 sec)

this pr(setting tidb_opt_join_reorder_threshold as 64): (nation supplier) (part lineitem)

mysql> set session tidb_opt_join_reorder_threshold = 64;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> explain select     nation,     o_year,     sum(amount) as sum_profit from     (         select             n_name as nation,             extract(year from o_orderdate) as o_year,             l_extendedprice * (1 - l_discount) - ps_supplycost * l_quantity as amount         from             part,             supplier,             lineitem,             partsupp,
  orders,             nation         where             s_suppkey = l_suppkey             and ps_suppkey = l_suppkey             and ps_partkey = l_partkey             and p_partkey = l_partkey             and o_orderkey = l_orderkey             and s_nationkey = n_nationkey             and p_name like '%dim%'     ) as profit group by     nation,     o_year order by     nation,     o_year desc;
+-------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                                                                                  | estRows      | task         | access object  | operator info                                                                                                                                                                                                                          |
+-------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Sort_79                                                                             | 2406.00      | root         |                | test.nation.n_name, Column#57:desc                                                                                                                                                                                                     |
| └─TableReader_388                                                                   | 2406.00      | root         |                | MppVersion: 3, data:ExchangeSender_387                                                                                                                                                                                                 |
|   └─ExchangeSender_387                                                              | 2406.00      | mpp[tiflash] |                | ExchangeType: PassThrough                                                                                                                                                                                                              |
|     └─Projection_82                                                                 | 2406.00      | mpp[tiflash] |                | test.nation.n_name, Column#57, Column#59                                                                                                                                                                                               |
|       └─Projection_383                                                              | 2406.00      | mpp[tiflash] |                | Column#59, test.nation.n_name, Column#57                                                                                                                                                                                               |
|         └─HashAgg_384                                                               | 2406.00      | mpp[tiflash] |                | group by:Column#70, test.nation.n_name, funcs:sum(Column#71)->Column#59, funcs:firstrow(test.nation.n_name)->test.nation.n_name, funcs:firstrow(Column#70)->Column#57, stream_count: 72                                                |
|           └─ExchangeReceiver_386                                                    | 2406.00      | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|             └─ExchangeSender_385                                                    | 2406.00      | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.nation.n_name, collate: utf8mb4_bin], stream_count: 72                                                                                                          |
|               └─HashAgg_381                                                         | 2406.00      | mpp[tiflash] |                | group by:Column#75, Column#76, funcs:sum(Column#74)->Column#71                                                                                                                                                                         |
|                 └─Projection_391                                                    | 247796839.16 | mpp[tiflash] |                | minus(mul(test.lineitem.l_extendedprice, minus(1, test.lineitem.l_discount)), mul(test.partsupp.ps_supplycost, test.lineitem.l_quantity))->Column#74, test.nation.n_name->Column#75, extract(YEAR, test.orders.o_orderdate)->Column#76 |
|                   └─Projection_370                                                  | 247796839.16 | mpp[tiflash] |                | test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.partsupp.ps_supplycost, test.orders.o_orderdate, test.nation.n_name                                                                            |
|                     └─Projection_360                                                | 247796839.16 | mpp[tiflash] |                | test.orders.o_orderdate, test.partsupp.ps_supplycost, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.nation.n_name, test.lineitem.l_orderkey                                                  |
|                       └─HashJoin_359                                                | 247796839.16 | mpp[tiflash] |                | inner join, equal:[eq(test.orders.o_orderkey, test.lineitem.l_orderkey)], stream_count: 72                                                                                                                                             |
|                         ├─ExchangeReceiver_110(Build)                               | 75000000.00  | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                         │ └─ExchangeSender_109                                      | 75000000.00  | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.orders.o_orderkey, collate: binary], stream_count: 72                                                                                                           |
|                         │   └─TableFullScan_108                                     | 75000000.00  | mpp[tiflash] | table:orders   | keep order:false                                                                                                                                                                                                                       |
|                         └─ExchangeReceiver_140(Probe)                               | 244189657.27 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                           └─ExchangeSender_139                                      | 244189657.27 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_orderkey, collate: binary]                                                                                                                           |
|                             └─Projection_138                                        | 244189657.27 | mpp[tiflash] |                | test.partsupp.ps_supplycost, test.lineitem.l_orderkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.nation.n_name, test.lineitem.l_suppkey, test.lineitem.l_partkey                         |
|                               └─HashJoin_111                                        | 244189657.27 | mpp[tiflash] |                | inner join, equal:[eq(test.partsupp.ps_suppkey, test.lineitem.l_suppkey) eq(test.partsupp.ps_partkey, test.lineitem.l_partkey)], stream_count: 72                                                                                      |
|                                 ├─ExchangeReceiver_114(Build)                       | 40000000.00  | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                 │ └─ExchangeSender_113                              | 40000000.00  | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.partsupp.ps_suppkey, collate: binary], [name: test.partsupp.ps_partkey, collate: binary], stream_count: 72                                                      |
|                                 │   └─TableFullScan_112                             | 40000000.00  | mpp[tiflash] | table:partsupp | keep order:false                                                                                                                                                                                                                       |
|                                 └─ExchangeReceiver_137(Probe)                       | 241498491.89 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                   └─ExchangeSender_136                              | 241498491.89 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_suppkey, collate: binary], [name: test.lineitem.l_partkey, collate: binary]                                                                          |
|                                     └─Projection_135                                | 241498491.89 | mpp[tiflash] |                | test.lineitem.l_orderkey, test.lineitem.l_partkey, test.lineitem.l_suppkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount, test.nation.n_name                                                      |
|                                       └─HashJoin_115                                | 241498491.89 | mpp[tiflash] |                | inner join, equal:[eq(test.lineitem.l_suppkey, test.supplier.s_suppkey)], stream_count: 72                                                                                                                                             |
|                                         ├─ExchangeReceiver_134(Build)               | 500000.00    | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                         │ └─ExchangeSender_133                      | 500000.00    | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.supplier.s_suppkey, collate: binary], stream_count: 72                                                                                                          |
|                                         │   └─Projection_132                        | 500000.00    | mpp[tiflash] |                | test.supplier.s_suppkey, test.nation.n_name                                                                                                                                                                                            |
|                                         │     └─HashJoin_127                        | 500000.00    | mpp[tiflash] |                | inner join, equal:[eq(test.supplier.s_nationkey, test.nation.n_nationkey)]                                                                                                                                                             |
|                                         │       ├─ExchangeReceiver_131(Build)       | 25.00        | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                         │       │ └─ExchangeSender_130              | 25.00        | mpp[tiflash] |                | ExchangeType: Broadcast, Compression: FAST                                                                                                                                                                                             |
|                                         │       │   └─TableFullScan_129             | 25.00        | mpp[tiflash] | table:nation   | keep order:false                                                                                                                                                                                                                       |
|                                         │       └─TableFullScan_128(Probe)          | 500000.00    | mpp[tiflash] | table:supplier | keep order:false                                                                                                                                                                                                                       |
|                                         └─ExchangeReceiver_126(Probe)               | 240833888.04 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                           └─ExchangeSender_125                      | 240833888.04 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_suppkey, collate: binary]                                                                                                                            |
|                                             └─Projection_124                        | 240833888.04 | mpp[tiflash] |                | test.lineitem.l_orderkey, test.lineitem.l_partkey, test.lineitem.l_suppkey, test.lineitem.l_quantity, test.lineitem.l_extendedprice, test.lineitem.l_discount                                                                          |
|                                               └─HashJoin_116                        | 240833888.04 | mpp[tiflash] |                | inner join, equal:[eq(test.part.p_partkey, test.lineitem.l_partkey)], stream_count: 72                                                                                                                                                 |
|                                                 ├─ExchangeReceiver_120(Build)       | 8000000.00   | mpp[tiflash] |                | stream_count: 72                                                                                                                                                                                                                       |
|                                                 │ └─ExchangeSender_119              | 8000000.00   | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.part.p_partkey, collate: binary], stream_count: 72                                                                                                              |
|                                                 │   └─Selection_118                 | 8000000.00   | mpp[tiflash] |                | like(test.part.p_name, "%dim%", 92)                                                                                                                                                                                                    |
|                                                 │     └─TableFullScan_117           | 10000000.00  | mpp[tiflash] | table:part     | keep order:false                                                                                                                                                                                                                       |
|                                                 └─ExchangeReceiver_123(Probe)       | 300005811.00 | mpp[tiflash] |                |                                                                                                                                                                                                                                        |
|                                                   └─ExchangeSender_122              | 300005811.00 | mpp[tiflash] |                | ExchangeType: HashPartition, Compression: FAST, Hash Cols: [name: test.lineitem.l_partkey, collate: binary]                                                                                                                            |
|                                                     └─TableFullScan_121             | 300005811.00 | mpp[tiflash] | table:lineitem | keep order:false                                                                                                                                                                                                                       |
+-------------------------------------------------------------------------------------+--------------+--------------+----------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
46 rows in set (0.03 sec)

No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

Performance
- Refined join query optimization: the optimizer now checks an additional threshold before using the advanced join-reorder path and otherwise falls back to the recursive optimizer, giving more predictable and controllable query planning behavior.

Signed-off-by: guo-shaoge <shaoge1994@163.com>

pantheon-ai · 2026-03-17T05:23:21Z

Review Complete (Fresh Run)

Findings: 0 issues
Posted: 0
Duplicates/Skipped: 0

✅ Code looks good - no P0/P1/P2 issues found in this review cycle.

_{ℹ️ Learn more details on Pantheon AI.}

coderabbitai · 2026-03-17T05:23:40Z

📝 Walkthrough

Walkthrough

Added a threshold check to JoinReOrderSolver: the direct call to joinorder.Optimize now requires both TiDBOptEnableAdvancedJoinReorder and TiDBOptJoinReorderThreshold <= 0; otherwise the optimizer falls back to the recursive optimization path.

Changes

Cohort / File(s)	Summary
Join Reorder Logic `pkg/planner/core/rule_join_reorder.go`	Introduced a gating condition that requires `TiDBOptJoinReorderThreshold <= 0` in addition to `TiDBOptEnableAdvancedJoinReorder` before calling `joinorder.Optimize`; otherwise uses the existing recursive optimizer path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

sessionctx/variable: add sysvar for new join reorder impl #66792: Adds runtime branching on TiDBOptEnableAdvancedJoinReorder to select join reorder implementation—closely related to the gating logic change.
planner: enable cd-c algorithm by default for join reorder | tidb-test=pr/2676 #66349: Prior PR referenced by linked issue that introduced a performance regression in TPCH; relevant for evaluating behavioral impact of join reorder changes.

Suggested reviewers

AilinKid
yudongusa

Poem

🐰 I hopped through code with careful cheer,
A threshold guard now sits quite near.
If zero smiles, the fast path plays,
Else recursion trims its ways.
Hooray — joins bound lightly, never fear! 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: disabling the new join reorder implementation when dynamic programming (DP) is enabled via tidb_opt_join_reorder_threshold.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#67073` by implementing a gating condition to disable the new join reorder when TiDBOptJoinReorderThreshold <= 0, restoring DP-based join ordering when DP is explicitly enabled.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to addressing the linked issue: the gating condition added to rule_join_reorder.go controls when advanced join reorder is used, with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description provides a clear problem statement (issue `#67073`), explains the root cause, documents the expected behavior change, and includes manual test results demonstrating the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Mini256

Code Review Complete

I've completed a thorough review of this PR. Here are my findings:

Summary

The code change is logically correct from a correctness standpoint. However, there are several important issues that should be addressed:

P1 Issues (High Priority)

Missing Documentation (pkg/planner/core/rule_join_reorder.go:291)
- The condition change lacks explanation of why advanced join reorder is incompatible with DP algorithm
- The relationship between TiDBOptEnableAdvancedJoinReorder and TiDBOptJoinReorderThreshold is non-obvious
- Suggestion: Add inline comment explaining: "Only use advanced join reorder when DP is disabled (threshold=0). When threshold>0, DP may be used for small join groups, which conflicts with the advanced join reorder framework."
Missing Test Coverage
- This PR changes core optimization behavior but includes no tests
- Critical scenarios to test:
  - Setting threshold > 0 should disable advanced join reorder
  - Default behavior (threshold=0) should continue using advanced join reorder
  - Various combinations of both variables should produce expected algorithm selection
- Without tests, this behavior could regress unnoticed
Logic Clarity Concern
- When TiDBOptJoinReorderThreshold > 0, advanced join reorder is completely disabled
- However, the old code path may still use DP for small groups (where joinGroupNum <= threshold)
- While this appears intentional, it should be documented to avoid confusion

P2 Issues (Process/Quality)

Incomplete PR Description
- PR body has template placeholders: "Issue Number: close #xxx"
- Missing: problem summary, implementation details, test plan
- This violates TiDB contribution guidelines and makes review difficult
- Please fill in: linked issue, problem description, solution explanation, and test plan
Code Style - Minor
- TiDBOptJoinReorderThreshold <= 0 could be == 0 since the variable is unsigned (MinValue: 0)
- Current form works but == 0 would be more explicit

Verdict

✅ Correctness: The code change is correct and safe from a logic perspective
⚠️ Process: Please address the P1 issues before merging, especially tests and documentation

🤖 Review powered by Pantheon AI

Mini256 · 2026-03-17T05:44:41Z

✅ Review Complete

I've completed a thorough code review of this PR. Here's the summary:

Findings

Correctness: ✅ Code is logically correct and safe
Issues Found: 3 P1 (high priority) + 2 P2 (process/quality)

P1 Issues (Must Address Before Merge)

Missing Documentation - Add comments explaining why advanced join reorder conflicts with DP algorithm (line 291)
Missing Test Coverage - No tests for this critical optimization behavior change
Logic Clarity - Document the interaction between TiDBOptEnableAdvancedJoinReorder and TiDBOptJoinReorderThreshold

P2 Issues (Process)

Incomplete PR Description - Fill in the template (issue reference, problem summary, test plan)
Code Style - Minor: <= 0 could be == 0 for clarity (unsigned variable)

Recommendation

The code change itself is correct, but please add tests and documentation before merging.

🤖 Detailed review posted above

codecov · 2026-03-17T06:07:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.4401%. Comparing base (319a683) to head (2f86341).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67079        +/-   ##
================================================
- Coverage   77.6914%   77.4401%   -0.2513%     
================================================
  Files          2016       1936        -80     
  Lines        551900     538705     -13195     
================================================
- Hits         428779     417174     -11605     
- Misses       121375     121505       +130     
+ Partials       1746         26      -1720

Flag	Coverage Δ
integration	`40.9869% <100.0000%> (-7.1421%)`	⬇️
unit	`76.5704% <100.0000%> (+0.3363%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`56.7974% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`48.8311% <ø> (-12.0263%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fixdb

+1

pantheon-ai · 2026-03-17T07:44:20Z

 // Optimize implements the base.LogicalOptRule.<0th> interface.
 func (s *JoinReOrderSolver) Optimize(_ context.Context, p base.LogicalPlan) (base.LogicalPlan, bool, error) {
-	if p.SCtx().GetSessionVars().TiDBOptEnableAdvancedJoinReorder {
+	if p.SCtx().GetSessionVars().TiDBOptEnableAdvancedJoinReorder && p.SCtx().GetSessionVars().TiDBOptJoinReorderThreshold <= 0 {


[P1] Missing Documentation

Why: The condition change lacks explanation of why advanced join reorder is incompatible with DP algorithm. The relationship between TiDBOptEnableAdvancedJoinReorder and TiDBOptJoinReorderThreshold is non-obvious.

Evidence: The added condition TiDBOptJoinReorderThreshold <= 0 at line 291 determines when to use advanced join reorder, but there's no comment explaining that this prevents conflicts with DP algorithm when threshold > 0.

Recommendation: Add inline comment: "Only use advanced join reorder when DP is disabled (threshold=0). When threshold>0, DP may be used for small join groups, which conflicts with the advanced join reorder framework."

hawkingrei · 2026-03-17T08:41:35Z


 // Optimize implements the base.LogicalOptRule.<0th> interface.
 func (s *JoinReOrderSolver) Optimize(_ context.Context, p base.LogicalPlan) (base.LogicalPlan, bool, error) {
-	if p.SCtx().GetSessionVars().TiDBOptEnableAdvancedJoinReorder {


add some test cases？

pantheon-ai

✅ Code looks good. No issues found.

winoros

The corresponding changed test in

ti-chi-bot · 2026-03-17T12:26:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fixdb, winoros

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/planner/OWNERS~~ [fixdb,winoros]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-03-17T12:26:14Z

[LGTM Timeline notifier]

Timeline:

2026-03-17 06:49:53.998294354 +0000 UTC m=+253321.085951891: ☑️ agreed by fixdb.
2026-03-17 12:26:13.070510571 +0000 UTC m=+273500.158168108: ☑️ agreed by winoros.

planner/core: disable new join reorder when DP is enabled

d6b40aa

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked release-note-none Denotes a PR that doesn't merit a release note. labels Mar 17, 2026

ti-chi-bot Bot added sig/planner SIG: Planner size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 17, 2026

Mini256 reviewed Mar 17, 2026

View reviewed changes

ti-chi-bot Bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked do-not-merge/needs-triage-completed labels Mar 17, 2026

comment

2f86341

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fixdb approved these changes Mar 17, 2026

View reviewed changes

ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 17, 2026

pantheon-ai Bot reviewed Mar 17, 2026

View reviewed changes

hawkingrei reviewed Mar 17, 2026

View reviewed changes

pantheon-ai Bot reviewed Mar 17, 2026

View reviewed changes

winoros approved these changes Mar 17, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 17, 2026

ti-chi-bot Bot merged commit d7cb925 into pingcap:master Mar 17, 2026
35 checks passed

coderabbitai Bot mentioned this pull request Mar 18, 2026

planner: move join reorder exploration into memo-lite #67121

Open

13 tasks

guo-shaoge mentioned this pull request Mar 25, 2026

planner/core: implement DP join reorder algorithm #67284

Merged

13 tasks

This was referenced Mar 25, 2026

planner: enable cd-c algorithm by default for join reorder | tidb-test=pr/2676 (#66349) #67289

Open

pkg/planner: add order-aware logical join reorder rule #67305

Merged

This was referenced Apr 13, 2026

planner: handle the projection between the join group #65367

Merged

planner: preserve index join hint warning after advanced join reorder #67838

Open

Conversation

guo-shaoge commented Mar 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

pantheon-ai Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Mini256 left a comment

Choose a reason for hiding this comment

Code Review Complete

Summary

P1 Issues (High Priority)

P2 Issues (Process/Quality)

Verdict

Uh oh!

Mini256 commented Mar 17, 2026

Findings

P1 Issues (Must Address Before Merge)

P2 Issues (Process)

Recommendation

Uh oh!

codecov Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fixdb left a comment

Choose a reason for hiding this comment

Uh oh!

pantheon-ai Bot Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

hawkingrei Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

pantheon-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

winoros left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Mar 17, 2026

Uh oh!

ti-chi-bot Bot commented Mar 17, 2026

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

guo-shaoge commented Mar 17, 2026 •

edited by coderabbitai Bot

Loading

pantheon-ai Bot commented Mar 17, 2026 •

edited

Loading

coderabbitai Bot commented Mar 17, 2026 •

edited

Loading

codecov Bot commented Mar 17, 2026 •

edited

Loading