planner: check view recursion when building source from view #20398

francis0407 · 2020-10-11T08:42:22Z

What problem does this PR solve?

Issue Number: close #20368

Problem Summary:

create table a (a int);
create view b as select * from a;
create view c as select * from b;
drop table a;
rename table c to a;
select * from a;

TiDB server crashed with stack overflow when building source from recursive view.

What is changed and how it works?

What's Changed:

Add a stack to record the views which are on building.
Replace the error ErrNoSuchTable with ErrViewRecursive

How it Works:

When we build source from view, we check if the view has already been on the build stack. If so, the view is defined recursively, and we return an ErrViewRecursive error.
If the view is not on the stack, we push it the stack, and pop it after building.

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test

Side effects

Breaking backward compatibility. This PR makes the error message compatible with MySQL.

Release note

fix stack overflow when building recursive view.

francis0407 · 2020-10-11T08:48:37Z

/label sig/planner,/bug-fix

ti-srebot · 2020-10-11T08:48:41Z

These labels are not found /bug-fix.

francis0407 · 2020-10-11T08:49:53Z

/label sig/planner,type/bug-fix

XuHuaiyu · 2020-10-19T03:23:48Z

executor/ddl_test.go

@@ -273,11 +273,24 @@ func (s *testSuite6) TestCreateView(c *C) {
 	tk.MustExec("create definer='root'@'localhost' view v_nested as select * from test_v_nested")
 	tk.MustExec("create definer='root'@'localhost' view v_nested2 as select * from v_nested")
 	_, err = tk.Exec("create or replace definer='root'@'localhost' view v_nested as select * from v_nested2")
-	c.Assert(terror.ErrorEqual(err, plannercore.ErrNoSuchTable), IsTrue)
+	c.Assert(terror.ErrorEqual(err, plannercore.ErrViewRecursive), IsTrue)


It seems that this error message is not compatible with MySQL

XuHuaiyu · 2020-10-19T03:25:19Z

planner/core/logical_plan_builder.go

@@ -3237,6 +3236,20 @@ func (b *PlanBuilder) buildMemTable(_ context.Context, dbName model.CIStr, table

 // BuildDataSourceFromView is used to build LogicalPlan from view
 func (b *PlanBuilder) BuildDataSourceFromView(ctx context.Context, dbName model.CIStr, tableInfo *model.TableInfo) (LogicalPlan, error) {
+	viewFullName := dbName.L + "." + tableInfo.Name.L


Can we extract these code block as in individual function like checkRecrusiveView?

addressed, PTAL

…ested_view # Conflicts: # planner/core/errors.go

francis0407 · 2020-10-27T05:27:18Z

/run-all-tests

francis0407 · 2020-10-27T05:33:42Z

The test failed due to #20596

francis0407 · 2020-11-09T12:56:01Z

/run-all-tests

francis0407 · 2020-11-09T13:33:35Z

/run-unit-test

XuHuaiyu

LGTM

XuHuaiyu · 2020-11-11T06:43:29Z

PTAL @wshwsh12

wshwsh12

LGTM

wshwsh12 · 2020-11-12T02:33:03Z

/merge

ti-srebot · 2020-11-12T02:33:21Z

/run-all-tests

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-11-12T02:45:54Z

cherry pick to release-3.0 in PR #21000

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-11-12T02:47:09Z

cherry pick to release-4.0 in PR #21001

…#21001) Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

…#21000) Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

fix view recursion

4ac8522

francis0407 requested review from a team as code owners October 11, 2020 08:42

francis0407 requested review from wshwsh12 and winoros and removed request for a team October 11, 2020 08:42

github-actions bot added the sig/execution SIG execution label Oct 11, 2020

ti-srebot added the sig/planner SIG: Planner label Oct 11, 2020

ti-srebot added the type/bugfix This PR fixes a bug. label Oct 11, 2020

zz-jason requested a review from XuHuaiyu October 12, 2020 01:35

XuHuaiyu reviewed Oct 19, 2020

View reviewed changes

address comment

571921b

francis0407 force-pushed the check_nested_view branch from 0b53d72 to 571921b Compare October 27, 2020 04:25

francis0407 added 4 commits October 27, 2020 12:27

Merge branch 'master' of https://github.com/pingcap/tidb into check_n…

a0b7b0a

…ested_view # Conflicts: # planner/core/errors.go

add error message

8035bd0

fix fmt

e76e3df

fix errors.toml

9c2b364

francis0407 added 2 commits October 27, 2020 17:59

Merge branch 'master' into check_nested_view

847ce22

Merge branch 'master' into check_nested_view

6b10e16

francis0407 requested a review from XuHuaiyu October 28, 2020 11:08

ichn-hu mentioned this pull request Nov 3, 2020

Welcome to contribute #20804

Closed

Merge branch 'master' into check_nested_view

af1a99a

Merge branch 'master' into check_nested_view

902016d

wshwsh12 added needs-cherry-pick-3.0 labels Nov 10, 2020

XuHuaiyu reviewed Nov 11, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 11, 2020

Merge branch 'master' into check_nested_view

9bf2951

wshwsh12 reviewed Nov 12, 2020

View reviewed changes

ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 12, 2020

ti-srebot approved these changes Nov 12, 2020

View reviewed changes

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 12, 2020

ti-srebot merged commit f81a5d1 into pingcap:master Nov 12, 2020

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 12, 2020

cherry pick pingcap#20398 to release-3.0

f35a2ee

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Nov 12, 2020

planner: check view recursion when building source from view (#20398) #21000

Merged

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 12, 2020

cherry pick pingcap#20398 to release-4.0

d8e7665

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Nov 12, 2020

planner: check view recursion when building source from view (#20398) #21001

Merged

ti-srebot added a commit that referenced this pull request Nov 17, 2020

planner: check view recursion when building source from view (#20398) (…

431e8c5

…#21001) Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot added a commit that referenced this pull request Dec 10, 2020

planner: check view recursion when building source from view (#20398) (…

c59a152

…#21000) Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: check view recursion when building source from view #20398

planner: check view recursion when building source from view #20398

francis0407 commented Oct 11, 2020

francis0407 commented Oct 11, 2020

ti-srebot commented Oct 11, 2020

francis0407 commented Oct 11, 2020

XuHuaiyu Oct 19, 2020

francis0407 Oct 27, 2020

XuHuaiyu Oct 19, 2020

francis0407 Oct 27, 2020

francis0407 commented Oct 27, 2020

francis0407 commented Oct 27, 2020

francis0407 commented Nov 9, 2020

francis0407 commented Nov 9, 2020

XuHuaiyu left a comment

XuHuaiyu commented Nov 11, 2020

wshwsh12 left a comment

wshwsh12 commented Nov 12, 2020

ti-srebot commented Nov 12, 2020

ti-srebot commented Nov 12, 2020

ti-srebot commented Nov 12, 2020

planner: check view recursion when building source from view #20398

planner: check view recursion when building source from view #20398

Conversation

francis0407 commented Oct 11, 2020

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

francis0407 commented Oct 11, 2020

ti-srebot commented Oct 11, 2020

francis0407 commented Oct 11, 2020

XuHuaiyu Oct 19, 2020

Choose a reason for hiding this comment

francis0407 Oct 27, 2020

Choose a reason for hiding this comment

XuHuaiyu Oct 19, 2020

Choose a reason for hiding this comment

francis0407 Oct 27, 2020

Choose a reason for hiding this comment

francis0407 commented Oct 27, 2020

francis0407 commented Oct 27, 2020

francis0407 commented Nov 9, 2020

francis0407 commented Nov 9, 2020

XuHuaiyu left a comment

Choose a reason for hiding this comment

XuHuaiyu commented Nov 11, 2020

wshwsh12 left a comment

Choose a reason for hiding this comment

wshwsh12 commented Nov 12, 2020

ti-srebot commented Nov 12, 2020

ti-srebot commented Nov 12, 2020

ti-srebot commented Nov 12, 2020