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.

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

server: check LOAD DATA is into a base table (#20924) #21638

Merged
merged 7 commits into from
Jan 26, 2021

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #20924 to release-4.0


What problem does this PR solve?

Issue Number: Fixes #20880

Problem Summary:

What is changed and how it works?

What's Changed:

Before loading data using LOAD DATA, a check if performed to make sure that the table is of type base table.

How it Works:

I have a PR on pingcap/parser#1080 to add an explicity IsBaseTable() function, which I think will be useful in other cases such as infoschema.

Note: The error code is a generic one. TiDB does not allow insertable views (differing from MySQL), and MySQL does not support sequences.

Related changes

Check List

Tests

  • Unit test

Side effects

  • None

Release note

  • TiDB now checks to make sure that the LOAD DATA statement can only load data into base tables.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@nullnotnil you're already a collaborator in bot's repo.

Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 11, 2020
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 11, 2020
@ti-srebot ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Dec 11, 2020
@ti-srebot
Copy link
Contributor Author

@tangenta, @wjhuang2016, @ngaut, PTAL.

2 similar comments
@ti-srebot
Copy link
Contributor Author

@tangenta, @wjhuang2016, @ngaut, PTAL.

@ti-srebot
Copy link
Contributor Author

@tangenta, @wjhuang2016, @ngaut, PTAL.

@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Jan 7, 2021
@jebter jebter modified the milestones: 4.0.0, v4.0.11 Jan 7, 2021
@zz-jason zz-jason added the type/bugfix This PR fixes a bug. label Jan 25, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @qw4990, this branch cannot be merged without an approval of release maintainers.

@ti-srebot
Copy link
Contributor Author

@qw4990, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: sql-infra(slack).

@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot
Copy link
Contributor Author

Sorry @qw4990, this branch cannot be merged without an approval of release maintainers.

@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/run-all-tests

@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/rebuild

@qw4990 qw4990 merged commit 8463d28 into pingcap:release-4.0 Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server sig/sql-infra SIG: SQL Infra status/LGT3 The PR has already had 3 LGTM. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants