Add hint for broadcast join. by hanfei1991 · Pull Request #818 · pingcap/parser

hanfei1991 · 2020-04-20T14:19:28Z

What problem does this PR solve?

Add hint for broadcast join.

What is changed and how it works?

Let parser support two new hints:

BC_JOIN(...)
TIDB_BCJ(...)

BC meas broadcast, which is brief and easy to understand :)

Check List

Tests

Unit test

Code changes

only change parser

Side effects

on side effects

Related changes

Need to cherry-pick to the release branch
Need to update the documentation
Need to be included in the release note

CLAassistant · 2020-04-20T14:19:39Z

All committers have signed the CLA.

codecov · 2020-04-20T14:21:54Z

Codecov Report

Merging #818 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #818   +/-   ##
=======================================
  Coverage   78.33%   78.33%           
=======================================
  Files          40       40           
  Lines       14765    14765           
=======================================
  Hits        11566    11566           
  Misses       2511     2511           
  Partials      688      688

kennytm

lgtm.

is there a corresponding tidb pr?

merge master

fix compile error

Add perfer local hint for broadcast join

zz-jason · 2020-05-11T04:22:19Z

ast/misc.go

 	case "max_execution_time":
 		ctx.WritePlainf("%d", n.HintData.(uint64))
-	case "tidb_hj", "tidb_smj", "tidb_inlj", "hash_join", "merge_join", "inl_join", "bc_join", "tidb_bcj":
+	case "tidb_hj", "tidb_smj", "tidb_inlj", "hash_join", "merge_join", "inl_join", "bc_join", "bcj_local", "tidb_bcj":


Nit: according to https://www.abbreviations.com/abbreviation/BROADCAST, seems BRDCST is a proper abbreviation for broadcast

It seems brdcst is for media, I think it's quite verbose. Can we make it brief and easy to write ?

if you shorten it to "brdcst" i'd rather you just spell it out entirely

select /*+ broadcast_join(...) */ ...

@hanfei1991 @lzmhhh123 PTAL the comment from @kennytm

I support the bc_join as the abbreviation.

lzmhhh123

LGTM

lzmhhh123 · 2020-06-17T06:48:59Z

/merge

ti-srebot · 2020-06-17T06:49:00Z

Sorry @lzmhhh123, you don't have permission to trigger auto merge event on this branch. You are not a committer for the related sigs:ddl(slack).

zz-jason · 2020-06-17T07:28:01Z

ast/misc.go

 	case "max_execution_time":
 		ctx.WritePlainf("%d", n.HintData.(uint64))
-	case "tidb_hj", "tidb_smj", "tidb_inlj", "hash_join", "merge_join", "inl_join", "bc_join", "tidb_bcj":
+	case "tidb_hj", "tidb_smj", "tidb_inlj", "hash_join", "merge_join", "inl_join", "bc_join", "bcj_local", "tidb_bcj":


@hanfei1991 @lzmhhh123 PTAL the comment from @kennytm

hanfei1991 · 2020-06-17T08:28:08Z

@zz-jason PTAL

zz-jason

LGTM

ti-srebot · 2020-06-17T08:55:17Z

/run-all-tests

kennytm · 2020-06-17T09:21:08Z

just wanna note that both bc_join and broadcast_join are fine for me, but brdcst_join is bad.

ti-srebot · 2020-07-29T06:13:27Z

cherry pick to release-4.0 failed

* add hint for bc join * refine * add hint for bc join * refine * refine * refine * refine again * pass test * fix compile error * add prefer local * use bcj_local * change bc_join to broadcast_join Co-authored-by: xufei <xufeixw@mail.ustc.edu.cn> Co-authored-by: ichn-hu <zfhu16@fudan.edu.cn>

* add hint for bc join * refine * add hint for bc join * refine * refine * refine * refine again * pass test * fix compile error * add prefer local * use bcj_local * change bc_join to broadcast_join Co-authored-by: xufei <xufeixw@mail.ustc.edu.cn> Co-authored-by: ichn-hu <zfhu16@fudan.edu.cn> Co-authored-by: xufei <xufeixw@mail.ustc.edu.cn> Co-authored-by: ichn-hu <zfhu16@fudan.edu.cn>

* add hint for bc join * refine * add hint for bc join * refine * refine * refine * refine again * pass test * fix compile error * add prefer local * use bcj_local * change bc_join to broadcast_join Co-authored-by: xufei <xufeixw@mail.ustc.edu.cn> Co-authored-by: ichn-hu <zfhu16@fudan.edu.cn>

hanfei1991 added 9 commits March 7, 2020 19:07

add hint for bc join

8b60f33

refine

fda93fd

add hint for bc join

4aebfbd

refine

cc2bee1

refine

ee0ed27

refine

f4bcaaa

refine again

3c139eb

Merge branch 'master' into hanfei/bc_join

63e37f3

pass test

f9d90d7

hanfei1991 requested a review from a team April 20, 2020 14:19

kennytm reviewed Apr 21, 2020

View reviewed changes

kennytm added the status/LGT1 LGT1 label Apr 21, 2020

windtalker and others added 8 commits May 6, 2020 17:59

Merge branch 'master' of https://github.com/pingcap/parser into bc_join

be293a8

Merge pull request #1 from windtalker/bc_join

497ab5e

merge master

fix compile error

b02b990

Merge pull request #2 from windtalker/bc_join

dde82b0

fix compile error

Merge branch 'master' into hanfei/bc_join

95b513a

add prefer local

6a681e8

use bcj_local

73e0eaa

Merge pull request #3 from ichn-hu/bc-gr-hint

9942139

Add perfer local hint for broadcast join

zz-jason reviewed May 11, 2020

View reviewed changes

hanfei1991 mentioned this pull request May 15, 2020

planner, executor: support broadcast join for tiflash engine. pingcap/tidb#17232

Merged

Merge branch 'master' into hanfei/bc_join

980888f

lzmhhh123 reviewed Jun 17, 2020

View reviewed changes

lzmhhh123 previously approved these changes Jun 17, 2020

View reviewed changes

lzmhhh123 added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jun 17, 2020

Merge branch 'master' into hanfei/bc_join

ac44a12

zz-jason requested changes Jun 17, 2020

View reviewed changes

change bc_join to broadcast_join

444a49f

hanfei1991 dismissed lzmhhh123’s stale review via 444a49f June 17, 2020 08:24

zz-jason approved these changes Jun 17, 2020

View reviewed changes

zz-jason added the status/can-merge label Jun 17, 2020

hanfei1991 merged commit e63134b into pingcap:master Jun 17, 2020

ti-srebot mentioned this pull request Jul 27, 2020

planner, executor: support broadcast join for tiflash engine. (#17232) pingcap/tidb#18801

Merged

hanfei1991 added the needs-cherry-pick-release-4.0 label Jul 29, 2020

hanfei1991 deleted the hanfei/bc_join branch July 29, 2020 06:12

Conversation

hanfei1991 commented Apr 20, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

CLAassistant commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kennytm left a comment

Choose a reason for hiding this comment

Uh oh!

zz-jason May 11, 2020

Choose a reason for hiding this comment

Uh oh!

hanfei1991 May 11, 2020

Choose a reason for hiding this comment

Uh oh!

kennytm May 11, 2020

Choose a reason for hiding this comment

Uh oh!

zz-jason Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

lzmhhh123 Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

lzmhhh123 left a comment

Choose a reason for hiding this comment

Uh oh!

lzmhhh123 commented Jun 17, 2020

Uh oh!

ti-srebot commented Jun 17, 2020

Uh oh!

zz-jason Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

hanfei1991 commented Jun 17, 2020

Uh oh!

zz-jason left a comment

Choose a reason for hiding this comment

Uh oh!

ti-srebot commented Jun 17, 2020

Uh oh!

kennytm commented Jun 17, 2020

Uh oh!

ti-srebot commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

CLAassistant commented Apr 20, 2020 •

edited

Loading

codecov bot commented Apr 20, 2020 •

edited

Loading