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

*: refine flashback syntax by remove until timestamp statement. #733

Merged
merged 5 commits into from
Feb 7, 2020

Conversation

crazycs520
Copy link
Contributor

@crazycs520 crazycs520 commented Feb 3, 2020

What problem does this PR solve?

PR #611 add the flashback syntax. But it was hard to use with the until timestamp statement.

And currently, we can't support flashback to any timestamp, even in the GC safe time.

After think over and talk with PM, we decided to remove the until timestamp statement in the flashback syntax. Now the flashback syntax is like below:

FLASHBACK TABLE table_name [TO] [other_name]

What is changed and how it works?

Check List

Tests

  • Unit test

@crazycs520 crazycs520 requested a review from a team February 3, 2020 11:28
@ghost ghost requested review from kennytm and removed request for a team February 3, 2020 11:28
@crazycs520 crazycs520 added the type/enhancement New feature or request label Feb 3, 2020
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 Feb 6, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM, please fix the integration test.

@crazycs520
Copy link
Contributor Author

@AilinKid Since this PR doesn't compatible, so it can't pass the integration test, it need merge with TiDB PR at the same time.

@AilinKid
Copy link
Contributor

AilinKid commented Feb 7, 2020

@AilinKid Since this PR doesn't compatible, so it can't pass the integration test, it need merge with TiDB PR at the same time.

Oh, got

@AilinKid AilinKid added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Feb 7, 2020
@crazycs520 crazycs520 merged commit d65f514 into pingcap:master Feb 7, 2020
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 LGT2 type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants