planner/core: simple greedy join reorder based on CBO #8394

winoros · 2018-11-22T05:00:49Z

What problem does this PR solve?

Let the default join reorder by greedy use CBO.

What is changed and how it works?

Removed old one, add the new greedy algo.

Check List

Tests

Unit test
Integration test
Manual test: TPCH performance checking.

Side effects

No

Related changes

Need to be included in the release note. Let default join reorder use CBO.

This change is

winoros · 2018-11-22T05:01:36Z

TPCH
Q5: cannot get result -> 55s
Q8: 46s->11s
Q17 66s->105s
Q18 80s->90s
Q21: 1min24s->24s

Need some investigation on Q17 and Q18.

ngaut · 2018-11-22T06:13:22Z

What's the results of star benchmark?

winoros · 2018-11-22T07:07:45Z

@ngaut TiDB haven't tested SSB yet. It will takes some time to do it.

winoros · 2018-11-22T11:47:17Z

Update for TPCH Q17:
Original join order is Join{Join{DataSource[2K rows], DataSource[60M rows]}[6w rows], Agg[2M rows]}[5K rows]

New order is Join{Join{DataSource[2K rows], Agg[2M rows]}[5K rows], DataSource[60M rows]}[5K rows]

You cannot say that the new order(don't care about the actual operator node below) is worse than before.
The problem is that the old one can let agg and inner join execute at the same time. While the new one will let the inner join waiting for agg's execution. And after that, begin to join with the 60M rows' DataSource. So it loses.

This can be optimized i think. But will leave it for future pr to do.

zz-jason · 2018-11-22T13:58:43Z

The problem here is we haven't consider the physical cost of the join order, only the number of rows of the intermediate result are considered in this greedy algorithm.

planner/core/rule_join_reorder_greedy.go

eurekaka

LGTM

planner/core/rule_join_reorder_dp.go

zz-jason · 2018-11-27T05:47:25Z

planner/core/rule_join_reorder_greedy.go

+)
+
+func extractInnerJoinGroup(p *LogicalJoin) ([]LogicalPlan, []*expression.ScalarFunction, []expression.Expression) {
+	if p.reordered || p.preferJoinType > uint(0) || p.JoinType != InnerJoin || p.StraightJoin {


Why stop extracting inner join groups when p.preferJoinType > uint(0)? IMHO, It's only a hint about join algorithm, not the join order.

Emm, we can do this.

OK. Could you please update code to remove this check in this function?

Oh, when do join reorder, we create new join node. And when creating it. We missed the join hint information. We need to change the way we hold it before we remove this check.

planner/core/rule_join_reorder_greedy.go

winoros · 2018-11-30T11:16:38Z

/run-all-tests tidb-test=pr/548

zz-jason · 2018-12-06T07:35:16Z

/run-all-tests

planner/core/rule_join_reorder_dp.go

zz-jason · 2018-12-06T07:53:00Z

planner/core/rule_join_reorder_greedy.go

+)
+
+func extractInnerJoinGroup(p *LogicalJoin) ([]LogicalPlan, []*expression.ScalarFunction, []expression.Expression) {
+	if p.reordered || p.preferJoinType > uint(0) || p.JoinType != InnerJoin || p.StraightJoin {


OK. Could you please update code to remove this check in this function?

planner/core/rule_join_reorder_greedy.go

…tidb into join-reorder-cbo-greedy

planner/core/rule_join_reorder_greedy.go

zz-jason

LGTM

zz-jason · 2018-12-07T07:31:31Z

/run-all-tests

zz-jason · 2018-12-07T08:18:20Z

/run-integration-common-test

winoros added 3 commits November 22, 2018 12:51

planner/core: join reorder based on CBO

5ddcee1

Merge branch 'master' into join-reorder-cbo-greedy

0d6a27f

remove old algo.

5329250

fix unit test

7907f79

zz-jason added type/enhancement The issue or PR belongs to an enhancement. sig/planner SIG: Planner labels Nov 22, 2018

eurekaka reviewed Nov 23, 2018

View reviewed changes

planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved

planner/core/rule_join_reorder_greedy.go Show resolved Hide resolved

planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved

planner/core/rule_join_reorder_greedy.go Show resolved Hide resolved

eurekaka reviewed Nov 23, 2018

View reviewed changes

eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 23, 2018

zz-jason reviewed Nov 27, 2018

View reviewed changes

address comments

4a5a0b8

Merge branch 'master' into join-reorder-cbo-greedy

6017f8b

zz-jason reviewed Dec 6, 2018

View reviewed changes

winoros added 3 commits December 6, 2018 19:05

address comments

fb0f192

Merge branch 'join-reorder-cbo-greedy' of https://github.com/winoros/…

dbdd073

…tidb into join-reorder-cbo-greedy

fix explain test

243b5db

zz-jason reviewed Dec 7, 2018

View reviewed changes

address comments

5080b6d

zz-jason approved these changes Dec 7, 2018

View reviewed changes

Merge branch 'master' into join-reorder-cbo-greedy

9f5a0d8

zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 7, 2018

winoros merged commit 6bd8bdd into pingcap:master Dec 7, 2018

winoros deleted the join-reorder-cbo-greedy branch December 7, 2018 08:34

iamzhoug37 pushed a commit to iamzhoug37/tidb that referenced this pull request Dec 13, 2018

planner/core: simple greedy join reorder based on CBO (pingcap#8394)

ab2edbd

kennytm mentioned this pull request Dec 27, 2018

Incorrect index after deleting from a table with index of multiple columns using a subquery, on latest master version (ADMIN CHECK TABLE failed) #8853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner/core: simple greedy join reorder based on CBO #8394

planner/core: simple greedy join reorder based on CBO #8394

winoros commented Nov 22, 2018 •

edited by c4pt0r

Loading

winoros commented Nov 22, 2018 •

edited

Loading

ngaut commented Nov 22, 2018

winoros commented Nov 22, 2018

winoros commented Nov 22, 2018

zz-jason commented Nov 22, 2018

eurekaka left a comment

zz-jason Nov 27, 2018

winoros Nov 29, 2018

zz-jason Dec 6, 2018

winoros Dec 6, 2018

winoros commented Nov 30, 2018

zz-jason commented Dec 6, 2018

zz-jason Dec 6, 2018

zz-jason left a comment

zz-jason commented Dec 7, 2018

zz-jason commented Dec 7, 2018

planner/core: simple greedy join reorder based on CBO #8394

planner/core: simple greedy join reorder based on CBO #8394

Conversation

winoros commented Nov 22, 2018 • edited by c4pt0r Loading

What problem does this PR solve?

What is changed and how it works?

Check List

winoros commented Nov 22, 2018 • edited Loading

ngaut commented Nov 22, 2018

winoros commented Nov 22, 2018

winoros commented Nov 22, 2018

zz-jason commented Nov 22, 2018

eurekaka left a comment

Choose a reason for hiding this comment

zz-jason Nov 27, 2018

Choose a reason for hiding this comment

winoros Nov 29, 2018

Choose a reason for hiding this comment

zz-jason Dec 6, 2018

Choose a reason for hiding this comment

winoros Dec 6, 2018

Choose a reason for hiding this comment

winoros commented Nov 30, 2018

zz-jason commented Dec 6, 2018

zz-jason Dec 6, 2018

Choose a reason for hiding this comment

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason commented Dec 7, 2018

zz-jason commented Dec 7, 2018

winoros commented Nov 22, 2018 •

edited by c4pt0r

Loading

winoros commented Nov 22, 2018 •

edited

Loading