Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

sql: fix where condition without brackets problem #372

Merged
merged 2 commits into from
Oct 14, 2021
Merged

sql: fix where condition without brackets problem #372

merged 2 commits into from
Oct 14, 2021

Conversation

lichunzhu
Copy link
Contributor

@lichunzhu lichunzhu commented Oct 14, 2021 •
edited
Loading

What problem does this PR solve?

close #371

What is changed and how it works?

If both conf.Where and where has a value, we use brackets to wrap these two conditions.

Check List

Tests

  • Unit test
  • Integration test
    Dumpling before this commit can't pass new integration tests.

Side effects

  • Possible performance regression

Related changes

  • Need to cherry-pick to the release branch

Release note

  • fix the problem that dumpling may dump extra data if --where is specified

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Oct 14, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • glorv

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.

glorv
glorv approved these changes Oct 14, 2021
View reviewed changes
Copy link
Collaborator

@glorv glorv left a comment

Choose a reason for hiding this comment

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

rest LGTM

v4/export/sql.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/LGT1 One reviewer approved (LGTM1) label Oct 14, 2021
Ehco1996
Ehco1996 approved these changes Oct 14, 2021
View reviewed changes
tests/basic/run.sh Outdated
@@ -40,6 +40,27 @@ expected=$(seq 3 9)
echo "expected ${expected}, actual ${actual}"
[ "$actual" = "$expected" ]

# Test for OR WHERE case. **Must dump MySQL here!!**
Copy link
Contributor

Choose a reason for hiding this comment

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

Must dump MySQL here!! What does this mean?🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about change these words to:
Better dump MySQL here because Dumpling has some special handle for concurrently dump TiDB tables.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@lichunzhu
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: e6d5341

@ti-chi-bot ti-chi-bot merged commit e754a70 into pingcap:master Oct 14, 2021
@ti-chi-bot ti-chi-bot mentioned this pull request Oct 14, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #373.

@lichunzhu lichunzhu deleted the fixWhereConditionProblem branch October 14, 2021 11:32
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #374.

This was referenced Oct 14, 2021
Merged
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #375.

@ti-chi-bot ti-chi-bot mentioned this pull request Oct 14, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #376.

ti-chi-bot added a commit that referenced this pull request Oct 15, 2021
@ti-chi-bot
sql: fix where condition without brackets problem (#372) (#373)
4024038
ti-chi-bot added a commit that referenced this pull request Oct 15, 2021
@ti-chi-bot
sql: fix where condition without brackets problem (#372) (#374)
7db623e
ti-chi-bot added a commit that referenced this pull request Oct 15, 2021
@ti-chi-bot
sql: fix where condition without brackets problem (#372) (#375)
341844e
ti-chi-bot added a commit that referenced this pull request Oct 15, 2021
@ti-chi-bot
sql: fix where condition without brackets problem (#372) (#376)
beddf63
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@lichunzhu
sql: fix where condition without brackets problem (pingcap#372)
bc77e84
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@lichunzhu
sql: fix where condition without brackets problem (pingcap#372)
f00f005
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@lichunzhu
sql: fix where condition without brackets problem (pingcap#372)
fb0da8d
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@lichunzhu
sql: fix where condition without brackets problem (pingcap#372)
a738afe
tisonkun pushed a commit to tisonkun/tidb that referenced this pull request Oct 20, 2021
@lichunzhu @tisonkun
sql: fix where condition without brackets problem (pingcap/dumpling#372)
c723369
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Ehco1996 Ehco1996 Ehco1996 approved these changes

@glorv glorv glorv approved these changes

@kennytm kennytm Awaiting requested review from kennytm

@lance6716 lance6716 Awaiting requested review from lance6716

Assignees
No one assigned
Labels
cherry-pick-approved needs-cherry-pick-release-4.0 needs-cherry-pick-release-5.0 needs-cherry-pick-release-5.1 needs-cherry-pick-release-5.2 Type: Need cherry pick to release-5.2 size/M status/can-merge status/LGT1 One reviewer approved (LGTM1)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dumpling may dump extra data if --where is specified
5 participants
@lichunzhu @ti-chi-bot @glorv @Ehco1996 @nongfushanquan