DNM: Release 6.5.12 Hotfix 20260601#12664
Conversation
Trim trailing whitespace and statement delimiters before checking whether an EXCHANGE PARTITION DDL ends with WITHOUT VALIDATION. Add test coverage for trailing whitespace, newlines, and semicolon variants to ensure the rebuilt DDL preserves WITHOUT VALIDATION.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request normalizes the DDL query string by trimming trailing whitespace, newlines, and semicolons before checking for the "WITHOUT VALIDATION" suffix in exchange partition actions, and adds comprehensive unit tests for these cases. The review feedback suggests adding defensive checks to prevent potential runtime panics from nil pointers or invalid substring indices.
| upperQuery := strings.ToUpper(job.Query) | ||
| normalizedUpperQuery := strings.TrimRight(upperQuery, " \t\r\n;") | ||
| idx1 := strings.Index(upperQuery, "EXCHANGE PARTITION") + len("EXCHANGE PARTITION") | ||
| idx2 := strings.Index(upperQuery, "WITH TABLE") |
There was a problem hiding this comment.
To prevent potential runtime panics, we should implement defensive programming checks here:
- Check if
tableInfoorpreTableInfoisnilbefore accessing their fields. - Ensure
EXCHANGE PARTITIONandWITH TABLEare actually found in the query (i.e.,strings.Indexdoes not return-1) and that their relative positions are valid (idx1 <= idx2) before slicing the query string.
If any of these checks fail, we can safely fall back to using the original job.Query.
if tableInfo == nil || preTableInfo == nil {
d.Query = job.Query
break
}
upperQuery := strings.ToUpper(job.Query)
normalizedUpperQuery := strings.TrimRight(upperQuery, " \t\r\n;")
idx1 := strings.Index(upperQuery, "EXCHANGE PARTITION")
idx2 := strings.Index(upperQuery, "WITH TABLE")
if idx1 == -1 || idx2 == -1 || idx1+len("EXCHANGE PARTITION") > idx2 {
d.Query = job.Query
break
}
idx1 += len("EXCHANGE PARTITION")|
/test pull-verify |
Resolve certificate paths relative to the test file instead of relying on git repo root discovery. This change is only to eliminate unrelated CI noise for this hotfix PR. Do not cherry-pick this commit.
|
/retest-required |
|
@haiboumich: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #xxx
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note