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

sync-diff-inspector: add sql-hint-use-index in config for automatically apply hint in check SQL #837

Merged
merged 32 commits into from
Mar 6, 2025

Conversation

joechenrh
Copy link
Contributor

@joechenrh joechenrh commented Jan 2, 2025

What problem does this PR solve?

Issue Number: close #836

Add hint /*+ USE_INDEX(dbName.tblName, indexName) */ to force index scan in checksum SQL. Note this index hint is only applied on TiDB source.

How to use?

[data-sources]
[data-sources.source]
host = "127.0.0.1"
port = 4000
user = "root"
password = ""

# Automatically detects and applies existing indexes, only applies on TiDB source.
sql-hint-use-index = "auto"

What is changed and how it works?

In each chunk, we will store the column names of the index used to split chunks. These column names will be used to match exact index name to build the index hint.

The reason we store column names rather than store index name directly is to avoid different index names between upstream and downstream.

For example:

  • upstream has table t1(id int, col1 int, unique index i1(id, col1))
  • downstream has table t1(id int, col1 int, unique index i2(id, col1))

Check List

Tests

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

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

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

@CLAassistant
Copy link

CLAassistant commented Jan 2, 2025

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot bot added the size/L label Jan 2, 2025
@ti-chi-bot ti-chi-bot bot added size/XL and removed size/L labels Jan 6, 2025
@ti-chi-bot ti-chi-bot bot added size/L and removed size/XL labels Jan 6, 2025
@joechenrh
Copy link
Contributor Author

/retest

1 similar comment
@joechenrh
Copy link
Contributor Author

/retest

@ti-chi-bot ti-chi-bot bot added size/XL and removed size/L labels Jan 20, 2025
@joechenrh
Copy link
Contributor Author

/release-note-none

@ti-chi-bot ti-chi-bot bot added size/XL and removed size/L labels Feb 27, 2025
@joechenrh
Copy link
Contributor Author

/retest

1 similar comment
@joechenrh
Copy link
Contributor Author

/retest

@joechenrh
Copy link
Contributor Author

/cc @kennytm

@ti-chi-bot ti-chi-bot bot requested a review from kennytm February 28, 2025 06:30
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.

rest lgtm

Comment on lines 124 to 127
require.Equal(t, len(node.ChunkRange.IndexColumnNames), len(testColNames))
for i, c := range testColNames {
require.Equal(t, c, node.ChunkRange.IndexColumnNames[i])
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
require.Equal(t, len(node.ChunkRange.IndexColumnNames), len(testColNames))
for i, c := range testColNames {
require.Equal(t, c, node.ChunkRange.IndexColumnNames[i])
}
require.Equal(t, node.ChunkRange.IndexColumnNames, testColNames)

require.Equal uses reflect.DeepEqual which supports slices.

Comment on lines 143 to 147
indexHint = fmt.Sprintf("/*+ USE_INDEX(`%s`.`%s`, `%s`) */",
matchSource.OriginSchema,
matchSource.OriginTable,
index.Name.L,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
indexHint = fmt.Sprintf("/*+ USE_INDEX(`%s`.`%s`, `%s`) */",
matchSource.OriginSchema,
matchSource.OriginTable,
index.Name.L,
)
indexHint = fmt.Sprintf("/*+ USE_INDEX(%s, %s) */",
dbutil.TableName(matchSource.OriginSchema, matchSource.OriginTable),
dbutil.ColumnName(index.Name.O),
)

(yeah it is not a column but whatever)

@joechenrh joechenrh changed the title sync-diff-inspector: add hint for check SQL sync-diff-inspector: add sql-hint-use-index in config for automatically apply hint in check SQL Feb 28, 2025
@joechenrh joechenrh requested a review from kennytm March 3, 2025 03:05
Copy link

ti-chi-bot bot commented Mar 6, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kennytm

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

The pull request process is described here

Needs approval from an approver in each of these files:

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 the lgtm label Mar 6, 2025
Copy link

ti-chi-bot bot commented Mar 6, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-03-06 07:58:43.916733346 +0000 UTC m=+514837.045653085: ☑️ agreed by kennytm.

@ti-chi-bot ti-chi-bot bot added the approved label Mar 6, 2025
@kennytm
Copy link
Contributor

kennytm commented Mar 6, 2025

/retest

@ti-chi-bot ti-chi-bot bot merged commit ac97e96 into pingcap:master Mar 6, 2025
3 checks passed
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.

sync_diff_inspector:Add hint use_index when verifying SQL using primary key/unique key paging
3 participants