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

parser: set UnionStmt and ShowStmt as readonly #755

Merged
merged 3 commits into from
Mar 1, 2020

Conversation

djshow832
Copy link
Contributor

What problem does this PR solve?

Fix pingcap/tidb#15050.
UNION statement and SHOW statement are not treated as readonly statement. These statements are executed again in retrying transactions.

What is changed and how it works?

Set UnionStmt and ShowStmt as readonly.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

N/A

Related changes

  • Need to cherry-pick to the release branch
  • Need to be included in the release note

Release Note

  • Treat UNION statement and SHOW statement as readonly statements.

@djshow832 djshow832 requested a review from a team March 1, 2020 07:24
@claassistantio
Copy link

claassistantio commented Mar 1, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Mar 1, 2020

Codecov Report

Merging #755 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #755   +/-   ##
=======================================
  Coverage   77.85%   77.85%           
=======================================
  Files          40       40           
  Lines       14455    14455           
=======================================
  Hits        11254    11254           
  Misses       2516     2516           
  Partials      685      685

@kennytm
Copy link
Contributor

kennytm commented Mar 1, 2020

But what about (select ... for update) union (select ... for update)?

@djshow832
Copy link
Contributor Author

But what about (select ... for update) union (select ... for update)?

Great catch!

Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm added the status/LGT1 LGT1 label Mar 1, 2020
@tiancaiamao
Copy link
Collaborator

LGTM

@kennytm kennytm added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Mar 1, 2020
@kennytm kennytm merged commit bfc519c into pingcap:master Mar 1, 2020
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Mar 1, 2020
* add UnionStmt and ShowStmt as readonly

* fix UnionStmt
@djshow832
Copy link
Contributor Author

/run-cherry-picker

@sre-bot
Copy link

sre-bot commented Mar 1, 2020

cherry pick to release-3.0 failed

djshow832 added a commit to djshow832/parser that referenced this pull request Mar 1, 2020
* add UnionStmt and ShowStmt as readonly

* fix UnionStmt
kennytm pushed a commit that referenced this pull request Mar 1, 2020
* add UnionStmt and ShowStmt as readonly

* fix UnionStmt
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
* add UnionStmt and ShowStmt as readonly

* fix UnionStmt
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
* add UnionStmt and ShowStmt as readonly

* fix UnionStmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Goroutine leak in transaction retry
5 participants