ddl: Drop a local temporary table #26063

Howie59 · 2021-07-08T11:40:02Z

What problem does this PR solve?

Issue Number: see #24169

What is changed and how it works?

Proposal: Temporary Table

Check List

Tests

Unit test

Release note

No release note

…/Howie59/tidb into drop-local-temp-table

executor/ddl.go

ddl/db_integration_test.go

executor/ddl.go

…nto drop-local-temp-table

executor/ddl.go

…nto drop-local-temp-table # Conflicts: # ddl/db_integration_test.go

djshow832 · 2021-07-13T13:51:07Z

executor/ddl.go

+			}
+		}
+		// if all tables are local temporary, directly drop those tables.
+		if len(s.Tables) == existsNum {


What if some are temporary tables while some are not?

it would be handled in dropTableObject, normal table would exec origin logic, local temp table handled singlely

How about drop all the temp tables here, and dropTableObject only drop normal tables?
It seems hard to keep the atomic of the drop statement, so the tables can be reordered.

but if table not exists, it should rollback. and the local temp tables should not drop

djshow832 · 2021-07-13T13:55:18Z

executor/ddl.go

+		}
+	}
+
+	if obt == tableObject && len(localTempTableList) > 0 {


This part is duplicate with DDLExec.Next, why?

if all tables are local temp table, it could be dropped directly, but if tables are consists of local temp and normal tables, it should check whether table exists, then drop it

djshow832 · 2021-07-14T11:17:30Z

executor/ddl.go

+			}
+		}
+		// if all tables are local temporary, directly drop those tables.
+		if len(s.Tables) == existsNum {


How about drop all the temp tables here, and dropTableObject only drop normal tables?
It seems hard to keep the atomic of the drop statement, so the tables can be reordered.

djshow832 · 2021-07-14T11:20:52Z

ddl/db_integration_test.go

@@ -2927,3 +2927,79 @@ func (s *testIntegrationSuite3) TestAvoidCreateViewOnLocalTemporaryTable(c *C) {
 	tk.MustExec("create view vv as select * from v0")
 	checkCreateView()
 }
+
+func (s *testIntegrationSuite3) TestDropTemporaryTable(c *C) {


Try this: drop table normal_table, temp_table;
Where the normal_table doesn't exist, which leads to rollback.
I assume it's both OK that temp_table exists or not.

executor/ddl.go

session/session.go

executor/ddl.go

ddl/db_integration_test.go

executor/ddl.go

…/Howie59/tidb into drop-local-temp-table

ddl/db_integration_test.go

executor/ddl.go

…/Howie59/tidb into drop-local-temp-table

Howie59 · 2021-07-20T04:43:32Z

/run-check_dev_2

ti-chi-bot · 2021-07-20T08:46:09Z

@lcwangchao: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

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.

ti-chi-bot · 2021-07-20T11:05:40Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

djshow832
tiancaiamao

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.

tiancaiamao · 2021-07-20T11:05:46Z

/merge

ti-chi-bot · 2021-07-20T11:05:49Z

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

Commit hash: ccdd722

drop local temp table

e6583f7

ti-chi-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 8, 2021

Merge branch 'master' into drop-local-temp-table

3233fef

github-actions bot added sig/execution SIG execution sig/sql-infra SIG: SQL Infra labels Jul 8, 2021

Howie59 added 3 commits July 9, 2021 10:19

add check

8e5596d

Merge branch 'drop-local-temp-table' of https://github.jparrowsec.cn.cnpmjs.org…

656002b

…/Howie59/tidb into drop-local-temp-table

Merge branch 'master' into drop-local-temp-table

7963dfd

lcwangchao reviewed Jul 9, 2021

View reviewed changes

executor/ddl.go Outdated Show resolved Hide resolved

ddl/db_integration_test.go Outdated Show resolved Hide resolved

lcwangchao mentioned this pull request Jul 9, 2021

Support temporary table #24169

Closed

89 tasks

lcwangchao requested review from tiancaiamao and djshow832 July 12, 2021 07:17

tiancaiamao reviewed Jul 13, 2021

View reviewed changes

ddl/db_integration_test.go Outdated Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

Howie59 added 2 commits July 13, 2021 13:12

Merge branch 'master' of https://github.jparrowsec.cn.cnpmjs.org/pingcap/tidb i…

b20380a

…nto drop-local-temp-table

opt drop operation

a326f5a

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 13, 2021

lcwangchao reviewed Jul 13, 2021

View reviewed changes

executor/ddl.go Outdated Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

remove data in membuffer

035ff97

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

Merge branch 'master' of https://github.jparrowsec.cn.cnpmjs.org/pingcap/tidb i…

7a5d2e6

…nto drop-local-temp-table # Conflicts: # ddl/db_integration_test.go

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

move package pos

a47e711

djshow832 reviewed Jul 13, 2021

View reviewed changes

djshow832 reviewed Jul 14, 2021

View reviewed changes

lcwangchao reviewed Jul 15, 2021

View reviewed changes

executor/ddl.go Show resolved Hide resolved

executor/ddl.go Show resolved Hide resolved

add tests

01191e4

lcwangchao reviewed Jul 16, 2021

View reviewed changes

session/session.go Outdated Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

executor/ddl.go Show resolved Hide resolved

executor/ddl.go Outdated Show resolved Hide resolved

ddl/db_integration_test.go Outdated Show resolved Hide resolved

Merge branch 'master' into drop-local-temp-table

b029036

lcwangchao reviewed Jul 16, 2021

View reviewed changes

executor/ddl.go Outdated Show resolved Hide resolved

Howie59 added 4 commits July 17, 2021 12:05

opt code

92d248b

Merge branch 'drop-local-temp-table' of https://github.jparrowsec.cn.cnpmjs.org…

ceccea1

…/Howie59/tidb into drop-local-temp-table

add err

b6302ca

Merge branch 'master' into drop-local-temp-table

affe6da

lcwangchao reviewed Jul 19, 2021

View reviewed changes

ddl/db_integration_test.go Show resolved Hide resolved

lcwangchao reviewed Jul 19, 2021

View reviewed changes

executor/ddl.go Outdated Show resolved Hide resolved

Howie59 and others added 4 commits July 20, 2021 11:15

fix tests

fee7f0c

Merge branch 'drop-local-temp-table' of https://github.jparrowsec.cn.cnpmjs.org…

7bfc433

…/Howie59/tidb into drop-local-temp-table

Merge branch 'master' into drop-local-temp-table

dcffa61

Merge branch 'master' into drop-local-temp-table

ccdd722

lcwangchao approved these changes Jul 20, 2021

View reviewed changes

djshow832 approved these changes Jul 20, 2021

View reviewed changes

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

tiancaiamao approved these changes Jul 20, 2021

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 20, 2021

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

Merge branch 'master' into drop-local-temp-table

166ef1f

ti-chi-bot merged commit dc8d0fd into pingcap:master Jul 20, 2021

Howie59 deleted the drop-local-temp-table branch July 20, 2021 12:15

This was referenced Jul 20, 2021

Drop table should clear data in session #25921

Closed

ddl: Drop a local temporary table #25885

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: Drop a local temporary table #26063

ddl: Drop a local temporary table #26063

Howie59 commented Jul 8, 2021 •

edited by lcwangchao

Loading

djshow832 Jul 13, 2021

Howie59 Jul 13, 2021

djshow832 Jul 14, 2021

Howie59 Jul 16, 2021

djshow832 Jul 13, 2021

Howie59 Jul 13, 2021

djshow832 Jul 14, 2021

djshow832 Jul 14, 2021

Howie59 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

tiancaiamao commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

ddl: Drop a local temporary table #26063

ddl: Drop a local temporary table #26063

Conversation

Howie59 commented Jul 8, 2021 • edited by lcwangchao Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Howie59 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

tiancaiamao commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

Howie59 commented Jul 8, 2021 •

edited by lcwangchao

Loading