Skip to content

schemastore: fix exchange partition name unquoting#4449

Merged
ti-chi-bot[bot] merged 1 commit into
masterfrom
ldz/fix-exchange-partion0312
Mar 12, 2026
Merged

schemastore: fix exchange partition name unquoting#4449
ti-chi-bot[bot] merged 1 commit into
masterfrom
ldz/fix-exchange-partion0312

Conversation

@lidezhu

@lidezhu lidezhu commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #4450

What is changed and how it works?

This pull request resolves a bug in the handling of EXCHANGE PARTITION DDL statements where partition names, especially those containing escaped backticks, were not being unquoted correctly. By implementing a robust UnquoteName utility and applying it to the relevant DDL parsing paths, the change ensures accurate processing of schema changes, preventing potential data inconsistencies or errors in schema replication.

Highlights

  • New Utility Function: Introduced a new UnquoteName utility function in the pkg/common package to correctly handle MySQL identifier unquoting, including the unescaping of doubled backticks.
  • DDL Parsing Fix: Integrated the UnquoteName function into the DDL handling logic for EXCHANGE PARTITION statements within the schemastore and event packages, ensuring proper parsing of partition names that may contain escaped backticks.
  • Improved Test Coverage: Updated existing unit tests and added new dedicated unit tests for the UnquoteName function to validate its correctness across various scenarios, including unquoted, quoted, and quoted names with escaped backticks.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note.

If you don't think this PR needs a release note then fill it with `None`.

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 12, 2026
@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the summary. You can try again by commenting /gemini summary.

@ti-chi-bot ti-chi-bot Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 12, 2026
@coderabbitai

coderabbitai Bot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@lidezhu has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 41 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 507e5cd0-212f-4875-8b06-10e046c9a016

📥 Commits

Reviewing files that changed from the base of the PR and between d37d22f and ae1f944.

📒 Files selected for processing (5)
  • logservice/schemastore/persist_storage_ddl_handlers.go
  • logservice/schemastore/persist_storage_test.go
  • pkg/common/event/util.go
  • pkg/common/table_info.go
  • pkg/common/table_info_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ldz/fix-exchange-partion0312

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lidezhu lidezhu changed the title schemastore: fix exchange partition schemastore: fix exchange partition name unquoting Mar 12, 2026
@lidezhu

lidezhu commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini summary

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

This pull request resolves a bug in the handling of EXCHANGE PARTITION DDL statements where partition names, especially those containing escaped backticks, were not being unquoted correctly. By implementing a robust UnquoteName utility and applying it to the relevant DDL parsing paths, the change ensures accurate processing of schema changes, preventing potential data inconsistencies or errors in schema replication.

Highlights

  • New Utility Function: Introduced a new UnquoteName utility function in the pkg/common package to correctly handle MySQL identifier unquoting, including the unescaping of doubled backticks.
  • DDL Parsing Fix: Integrated the UnquoteName function into the DDL handling logic for EXCHANGE PARTITION statements within the schemastore and event packages, ensuring proper parsing of partition names that may contain escaped backticks.
  • Improved Test Coverage: Updated existing unit tests and added new dedicated unit tests for the UnquoteName function to validate its correctness across various scenarios, including unquoted, quoted, and quoted names with escaped backticks.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • logservice/schemastore/persist_storage_ddl_handlers.go
    • Replaced manual backtick removal with a call to common.UnquoteName for parsing partition names in EXCHANGE PARTITION DDLs.
  • logservice/schemastore/persist_storage_test.go
    • Modified the TestBuildPersistedDDLEventEscapesIdentifiers test case to use a partition name with an escaped backtick (p``0), aligning with the new unquoting logic.
  • pkg/common/event/util.go
    • Replaced manual backtick removal with a call to common.UnquoteName for parsing partition names in EXCHANGE PARTITION DDLs within the DDL2Job function.
  • pkg/common/table_info.go
    • Added a new public function UnquoteName that removes one layer of MySQL identifier quoting and unescapes doubled backticks.
  • pkg/common/table_info_test.go
    • Added a new test file table_info_test.go containing TestUnquoteName with test cases for unquoted, quoted, and quoted with escaped backtick scenarios to validate the new UnquoteName function.
Activity
  • A bot encountered an error while attempting to create a summary.
  • A bot indicated that the pull request is not yet approved and listed charlescheung96 as an approver.
  • A bot reported that the author exceeded the rate limit for automated reviews.
  • The author manually requested a summary from the bot.

@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Mar 12, 2026

assert.Equal(t,
"ALTER TABLE `part``db`.`pt``x` EXCHANGE PARTITION `p0` WITH TABLE `normal``db`.`normal``t` WITHOUT VALIDATION",
"ALTER TABLE `part``db`.`pt``x` EXCHANGE PARTITION `p``0` WITH TABLE `normal``db`.`normal``t` WITHOUT VALIDATION",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ditto

Comment thread pkg/common/table_info.go
}

// UnquoteName removes one layer of MySQL identifier quoting and unescapes doubled backticks.
func UnquoteName(name string) string {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think there exists a similar function, and the function is duplicate

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

which one?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess 😆

t.Run("exchange partition", func(t *testing.T) {
job := buildExchangePartitionJobForTest(100, 200, 300, "pt`x", []int64{301}, 1000)
job.Query = "ALTER TABLE `ignored`.`ignored` EXCHANGE PARTITION `p0` WITH TABLE `ignored2`.`ignored2` WITHOUT VALIDATION"
job.Query = "ALTER TABLE `ignored`.`ignored` EXCHANGE PARTITION `p``0` WITH TABLE `ignored2`.`ignored2` WITHOUT VALIDATION"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it equal to p0?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No. It is p``0, this pr is to avoid corrupting names like this.

@ti-chi-bot

ti-chi-bot Bot commented Mar 12, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenfyzhong, wk989898

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [tenfyzhong,wk989898]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 12, 2026
@ti-chi-bot

ti-chi-bot Bot commented Mar 12, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-03-12 07:41:00.740562232 +0000 UTC m=+508692.252619893: ☑️ agreed by tenfyzhong.
  • 2026-03-12 08:19:01.718396771 +0000 UTC m=+510973.230454442: ☑️ agreed by wk989898.

@ti-chi-bot ti-chi-bot Bot merged commit 8a61ab3 into master Mar 12, 2026
12 checks passed
@ti-chi-bot ti-chi-bot Bot deleted the ldz/fix-exchange-partion0312 branch March 12, 2026 09:30
tenfyzhong pushed a commit that referenced this pull request Mar 18, 2026
close #4450

(cherry picked from commit 8a61ab3)
Signed-off-by: tenfyzhong <tenfy@tenfy.cn>
@lidezhu lidezhu added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Jun 8, 2026
@lidezhu

lidezhu commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator Author

/run-cherry-picker

@ti-chi-bot

Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #5247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exchange partition query rebuild corrupts partition names containing backticks

4 participants