Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner/core: make join reorder by dp work #8816

Merged
merged 24 commits into from
Apr 28, 2019

Conversation

winoros
Copy link
Member

@winoros winoros commented Dec 25, 2018

What problem does this PR solve?

Make the algo that use dp to do the join reorder integrated with the current planner.

What is changed and how it works?

The original algo only considers equal conditions. This time we add support for OhterConds.
And when the join node is no greater than 10, we'll choose it. Otherwise, we'll use the greedy one.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below) TPCH Testing is in preparing.

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Side effects

  • Increased code complexity

Related changes

  • Need to be included in the release note

This change is Reviewable

@winoros
Copy link
Member Author

winoros commented Dec 25, 2018

The impact on TPCH has not been validated.

@winoros
Copy link
Member Author

winoros commented Dec 26, 2018

/run-all-tests

@winoros
Copy link
Member Author

winoros commented Dec 26, 2018

/run-all-tests

@winoros
Copy link
Member Author

winoros commented Dec 26, 2018

/run-all-tests

@winoros winoros changed the title [WIP] planner/core: make join reorder by dp work planner/core: make join reorder by dp work Dec 29, 2018
planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved
planner/core/optimizer.go Outdated Show resolved Hide resolved
@@ -690,6 +695,8 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
s.EnableRadixJoin = TiDBOptOn(val)
case TiDBEnableWindowFunction:
s.EnableWindowFunction = TiDBOptOn(val)
case TiDBOptJoinOrderAlgoThreshold:
s.TiDBOptJoinOrderAlgoThreshold = tidbOptPositiveInt32(val, DefTiDBOptJoinOrderAlgoThreshold)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also limit its upper bound?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the set it too large can be treated as that we want every join order calculated by dp.
Though it not reasonable in a real environment, i think it's reasonable in logic...

Copy link
Member

Choose a reason for hiding this comment

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

If it is larger than 64, will the DP algorithm panic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Show resolved Hide resolved
planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved
@winoros winoros requested a review from alivxxx January 7, 2019 05:11
@eurekaka eurekaka added the type/enhancement The issue or PR belongs to an enhancement. label Jan 7, 2019
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM, please resolve conflicts.

@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 7, 2019
@alivxxx
Copy link
Contributor

alivxxx commented Jan 10, 2019

@winoros Any updates?

@winoros
Copy link
Member Author

winoros commented Mar 4, 2019

Disable by default.

@winoros winoros requested a review from zz-jason March 4, 2019 09:13
sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_greedy.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
sessionctx/variable/varsutil.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
sessionctx/variable/varsutil.go Outdated Show resolved Hide resolved
sessionctx/variable/session.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
planner/core/rule_join_reorder_dp.go Outdated Show resolved Hide resolved
@winoros winoros requested review from eurekaka and alivxxx and removed request for eurekaka April 24, 2019 07:27
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

zz-jason
zz-jason previously approved these changes Apr 24, 2019
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 24, 2019
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason merged commit 9630d57 into pingcap:master Apr 28, 2019
@winoros winoros deleted the join-reorder-dp branch May 14, 2019 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants