planner: add a hint for CTE #34574

dayicklp · 2022-05-11T09:41:57Z

What problem does this PR solve?

Issue Number: close #17472

Problem Summary: Insert a hint for CTE, which can control the merge function in CTE

What is changed and how it works?

Check List

Tests

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

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

I want to insert a hint for CTE-inline, but the Optimizer doesn't find the hint which I code.

ti-chi-bot · 2022-05-11T09:42:00Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

winoros
wjhuang2016

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

sre-bot · 2022-05-11T09:42:07Z

All committers have signed the CLA.

sre-bot · 2022-05-11T09:52:38Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/8089e5186b8b4188dab0b133a3daebca27084707

only insert some var

dayicklp · 2022-05-11T16:57:43Z

these is no unit test, I will finish it today.

fixdb · 2022-05-11T19:05:23Z

parser/parser_test.go

@@ -4192,6 +4192,16 @@ func TestOptimizerHints(t *testing.T) {
 	require.Len(t, hints, 2)
 	require.Equal(t, "limit_to_cop", hints[0].HintName.L)
 	require.Equal(t, "limit_to_cop", hints[1].HintName.L)
+
+	// Test CTE_INLINE
+	stmt, _, err = p.Parse("with cte(x) as (select /*+ CTE_INLINE(), cte_inline() */ * from t1) select * from cte;", "", "")


If there is multiple CTEs, do we need to write hint for each CTE to get every CTE inlined?

At present, I intend to add hint in each CTE, because hint is added after each “select”. If there is a better way, I will add it later.

I fix my mistake , and remake the hintparser.y

comment on exported type CTEHintInfo should be of the form "CTEHintInfo ..." (with optional leading

fix merge conflict.

try to add a unit test

dayicklp · 2022-07-15T07:57:56Z

/run-unit-test

dayicklp · 2022-07-15T08:45:57Z

/run-unit-test

dayicklp · 2022-07-18T08:08:07Z

/run-check_dev_2

dayicklp · 2022-07-18T09:21:58Z

/run-check_dev_2

wjhuang2016

Any tiflash tests?

planner/core/logical_plan_builder.go

dayicklp · 2022-07-18T11:37:33Z

Any tiflash tests?

we will code tiflash tests on automated-tests

dayicklp · 2022-07-19T02:47:06Z

@wjhuang2016 PTAL again

ti-chi-bot · 2022-07-19T05:59:41Z

@dayicklp: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

wjhuang2016 · 2022-07-19T06:01:01Z

/merge

ti-chi-bot · 2022-07-19T06:01:04Z

This pull request has been accepted and is ready to merge.

Commit hash: 2dc8025

sre-bot · 2022-07-19T07:06:56Z

TiDB MergeCI notify

🔴 Bad News! New failing [1] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-ddl-test	🟥 failed 1, success 5, total 6	14 min	New failing
idc-jenkins-ci/integration-cdc-test	🟢 all 36 tests passed	29 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 11 tests passed	26 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 12 tests passed	14 min	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	7 min 30 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	6 min 10 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	5 min 54 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	4 min 25 sec	Existing passed
idc-jenkins-ci-tidb/mybatis-test	🟢 all 1 tests passed	4 min 25 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

add a hint for CTE

c931243

I want to insert a hint for CTE-inline, but the Optimizer doesn't find the hint which I code.

ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels May 11, 2022

ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 11, 2022

add parser 's change for hint

cd8bb91

only insert some var

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2022

dayicklp marked this pull request as draft May 11, 2022 16:55

dayicklp marked this pull request as ready for review May 11, 2022 16:56

fixdb reviewed May 11, 2022

View reviewed changes

fix parser mistake for cteinline

f935e53

I fix my mistake , and remake the hintparser.y

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 12, 2022

dayicklp and others added 9 commits May 12, 2022 10:41

fix comment mistake

a453672

comment on exported type CTEHintInfo should be of the form "CTEHintInfo ..." (with optional leading

Update planbuilder.go

d4cc365

fix error

e38b66f

Update planbuilder.go

e2e88af

Update logical_plan_builder.go

03e8165

renew the code version

a10a8db

fix merge conflict.

Merge branch 'master' into cte_by_dyp

17e8a0c

Merge branch 'master' into cte_by_dyp

1ea8d91

Update parser_test.go

1f2064f

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2022

dayicklp and others added 4 commits May 12, 2022 23:18

Merge branch 'master' into cte_by_dyp

3fca3f6

Update physical_plan_test.go

afdf3a8

try to add a unit test

Update physical_plan_test.go

1888699

Update physical_plan_test.go

bf91604

dayicklp and others added 2 commits July 15, 2022 11:55

Update hintparser.go

22bc4f2

Merge branch 'master' into cte_by_dyp

f702e07

dayicklp mentioned this pull request Jul 15, 2022

planner: add a Merge hint for CTE inline #36240

Closed

12 tasks

Merge branch 'master' into cte_by_dyp

3744935

dayicklp mentioned this pull request Jul 15, 2022

planner: add a Merge hint for CTE inline #36245

Open

12 tasks

Merge branch 'master' into cte_by_dyp

d50a71b

dayicklp requested a review from winoros July 18, 2022 07:30

Merge branch 'master' into cte_by_dyp

2dc8025

winoros approved these changes Jul 18, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 18, 2022

wjhuang2016 reviewed Jul 18, 2022

View reviewed changes

planner/core/logical_plan_builder.go Show resolved Hide resolved

dayicklp requested a review from wjhuang2016 July 18, 2022 11:59

wjhuang2016 approved these changes Jul 19, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 19, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 19, 2022

Merge branch 'master' into cte_by_dyp

8089e51

ti-chi-bot merged commit a57dd37 into pingcap:master Jul 19, 2022

dayicklp mentioned this pull request Jul 20, 2022

planner: add binding and recursive test for Merge hint #36352

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: add a hint for CTE #34574

planner: add a hint for CTE #34574

dayicklp commented May 11, 2022 •

edited

Loading

ti-chi-bot commented May 11, 2022 •

edited

Loading

sre-bot commented May 11, 2022 •

edited

Loading

sre-bot commented May 11, 2022 •

edited

Loading

dayicklp commented May 11, 2022

fixdb May 11, 2022

dayicklp May 12, 2022

dayicklp commented Jul 15, 2022

dayicklp commented Jul 15, 2022

dayicklp commented Jul 18, 2022

dayicklp commented Jul 18, 2022

wjhuang2016 left a comment

dayicklp commented Jul 18, 2022

dayicklp commented Jul 19, 2022

ti-chi-bot commented Jul 19, 2022

wjhuang2016 commented Jul 19, 2022

ti-chi-bot commented Jul 19, 2022

sre-bot commented Jul 19, 2022

planner: add a hint for CTE #34574

planner: add a hint for CTE #34574

Conversation

dayicklp commented May 11, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented May 11, 2022 • edited Loading

sre-bot commented May 11, 2022 • edited Loading

sre-bot commented May 11, 2022 • edited Loading

dayicklp commented May 11, 2022

fixdb May 11, 2022

Choose a reason for hiding this comment

dayicklp May 12, 2022

Choose a reason for hiding this comment

dayicklp commented Jul 15, 2022

dayicklp commented Jul 15, 2022

dayicklp commented Jul 18, 2022

dayicklp commented Jul 18, 2022

wjhuang2016 left a comment

Choose a reason for hiding this comment

dayicklp commented Jul 18, 2022

dayicklp commented Jul 19, 2022

ti-chi-bot commented Jul 19, 2022

wjhuang2016 commented Jul 19, 2022

ti-chi-bot commented Jul 19, 2022

sre-bot commented Jul 19, 2022

TiDB MergeCI notify

dayicklp commented May 11, 2022 •

edited

Loading

ti-chi-bot commented May 11, 2022 •

edited

Loading

sre-bot commented May 11, 2022 •

edited

Loading

sre-bot commented May 11, 2022 •

edited

Loading