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.

Sign up for GitHub

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

Jump to bottom

*: support read and write operations for the global temporary table #24196

Merged
merged 19 commits into from
Apr 29, 2021
Merged

*: support read and write operations for the global temporary table #24196

merged 19 commits into from
Apr 29, 2021

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Problem Summary:

What is changed and how it works?

Proposal: temporary table

What's Changed:

How it Works:

Support read and write operations for the global temporary table is quite simple.
Just discard the key-value belong to the temporary table before commit.

Check List

Tests

  • Unit test

Release note

  • No release note

@tiancaiamao tiancaiamao requested a review from a team as a code owner April 22, 2021 03:05
@tiancaiamao tiancaiamao requested review from XuHuaiyu and removed request for a team April 22, 2021 03:05
@CLAassistant
Copy link

CLAassistant commented Apr 22, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 22, 2021
This was referenced Apr 22, 2021
Closed
Merged
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 23, 2021
@tiancaiamao
Copy link
Contributor Author

/rebuild

djshow832
djshow832 reviewed Apr 25, 2021
View reviewed changes
sessionctx/variable/session.go Outdated Show resolved Hide resolved
sessionctx/variable/session.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 26, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2021
@tiancaiamao
Copy link
Contributor Author

/run-check_dev_2

panic: test timed out after 9m0s

This CI is quite unstable currently.

@tiancaiamao tiancaiamao requested a review from a team as a code owner April 29, 2021 07:27
@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Apr 29, 2021
tiancaiamao
tiancaiamao commented Apr 29, 2021
View reviewed changes
executor/seqtest/seq_executor_test.go
@@ -967,7 +967,7 @@ func (s *seqTestSuite) TestBatchInsertDelete(c *C) {
atomic.StoreUint64(&kv.TxnTotalSizeLimit, originLimit)
}()
// Set the limitation to a small value, make it easier to reach the limitation.
atomic.StoreUint64(&kv.TxnTotalSizeLimit, 5000)
atomic.StoreUint64(&kv.TxnTotalSizeLimit, 5500)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is because we update the parser, and marshal table info use more bytes (add a new temporary field, now it's 5038 byte).
If I don't change here, CI will fail and complain transaction too large for the create table ... statement.

tiancaiamao
tiancaiamao commented Apr 29, 2021
View reviewed changes
ddl/column_type_change_test.go
@@ -233,7 +233,7 @@ func (s *testColumnTypeChangeSuite) TestRollbackColumnTypeChangeBetweenInteger(c
SQL := "alter table t modify column c2 int not null"
_, err := tk.Exec(SQL)
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "[ddl:1]MockRollingBackInCallBack-none")
c.Assert(err.Error(), Equals, "[ddl:1]MockRollingBackInCallBack-queueing")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is because we use the latest parser and it change the String() method
https://github.com/pingcap/parser/pull/1210/files#diff-a5ce247e55d5a20707517e34bef620f2eb369b4f9b8fa44fdfb2c18a702418e8R72

@coocood
Copy link
Member

coocood commented Apr 29, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 351d871

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 29, 2021
@tiancaiamao
Copy link
Contributor Author

/run-integration-common-test

@coocood
Copy link
Member

coocood commented Apr 29, 2021

/lgtm

@tiancaiamao
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

@ti-chi-bot
Copy link
Member

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

@ti-chi-bot
Copy link
Member

/run-all-tests

This bot automatically retries jobs that failed on can merge PRs (send feedback to hi-rustin).

Silence the bot with the /merge cancel comment for consistent failures.

@Rustin170506
Copy link
Member

/run-check_title

@Rustin170506
Copy link
Member

/run-check-title

@ti-chi-bot ti-chi-bot merged commit af6cece into pingcap:master Apr 29, 2021
@tiancaiamao tiancaiamao deleted the temporary-rw branch May 11, 2021 05:07
@tiancaiamao tiancaiamao mentioned this pull request May 19, 2021
Merged
@tiancaiamao tiancaiamao mentioned this pull request Jun 17, 2021
Merged
This was referenced Jul 23, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Howie59 Howie59 Howie59 left review comments

@djshow832 djshow832 djshow832 approved these changes

@coocood coocood coocood approved these changes

@AilinKid AilinKid Awaiting requested review from AilinKid

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

@morgo morgo Awaiting requested review from morgo

@tangenta tangenta Awaiting requested review from tangenta

@bb7133 bb7133 Awaiting requested review from bb7133

@lysu lysu Awaiting requested review from lysu

Assignees
No one assigned
Labels
component/session sig/sql-infra SIG: SQL Infra size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/new-feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@tiancaiamao @CLAassistant @morgo @coocood @djshow832 @ti-chi-bot @Rustin170506 @Howie59