br: compatible with and add integration test for partial index (#64434)#68829
br: compatible with and add integration test for partial index (#64434)#68829ti-chi-bot wants to merge 1 commit into
Conversation
Signed-off-by: Jianjun Liao <jianjun.liao@outlook.com>
|
[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 |
📝 WalkthroughWalkthroughThis PR adds support for partial indexes (indexes with WHERE conditions) to the TiDB backup/restore ingest repair flow. The implementation conditionally appends WHERE clauses to generated ALTER TABLE DDL, and comprehensive test cases validate correct creation and query behavior across index metadata and data lookups. ChangesPartial Index Condition Support
🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@br/tests/br_pitr/check/check_ingest_repair.sh`:
- Around line 143-145: The current assertion in check_ingest_repair.sh queries
INFORMATION_SCHEMA.TIDB_INDEXES using an allocation-dependent INDEX_ID > 2 which
is flaky; update the SQL used by run_sql to assert partial-index rows by stable
index identifiers (INDEX_NAME) and PREDICATE instead of INDEX_ID, e.g. filter
WHERE TABLE_SCHEMA='test' AND TABLE_NAME='pairs20' AND INDEX_NAME IN
('<partial_idx_name1>','<partial_idx_name2>') AND PREDICATE = '`id` > 5' (or use
a single INDEX_NAME = '<name>' if only one), then adjust the expected RESCNT
accordingly so the check is deterministic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bffc2494-a8a2-4f70-a397-8e18c93ecf8d
📒 Files selected for processing (4)
br/pkg/restore/log_client/client.gobr/tests/br_pitr/check/check_ingest_repair.shbr/tests/br_pitr/incremental_data/ingest_repair.sqlbr/tests/br_pitr/prepare_data/ingest_repair.sql
| ## check table test.pairs20 | ||
| run_sql "SELECT COUNT(*) AS RESCNT FROM INFORMATION_SCHEMA.TIDB_INDEXES WHERE TABLE_SCHEMA = 'test' AND TABLE_NAME = 'pairs20' AND INDEX_ID > 2 AND PREDICATE = '\`id\` > 5'" | ||
| check_contains "RESCNT: 2" |
There was a problem hiding this comment.
Make the partial-index metadata assertion deterministic.
Line 144’s INDEX_ID > 2 is allocation-dependent and can make this check flaky. Assert by key names instead.
Suggested fix
-run_sql "SELECT COUNT(*) AS RESCNT FROM INFORMATION_SCHEMA.TIDB_INDEXES WHERE TABLE_SCHEMA = 'test' AND TABLE_NAME = 'pairs20' AND INDEX_ID > 2 AND PREDICATE = '\`id\` > 5'"
+run_sql "SELECT COUNT(*) AS RESCNT FROM INFORMATION_SCHEMA.TIDB_INDEXES WHERE TABLE_SCHEMA = 'test' AND TABLE_NAME = 'pairs20' AND KEY_NAME IN ('i1','i2') AND PREDICATE = '\`id\` > 5'"
check_contains "RESCNT: 2"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@br/tests/br_pitr/check/check_ingest_repair.sh` around lines 143 - 145, The
current assertion in check_ingest_repair.sh queries
INFORMATION_SCHEMA.TIDB_INDEXES using an allocation-dependent INDEX_ID > 2 which
is flaky; update the SQL used by run_sql to assert partial-index rows by stable
index identifiers (INDEX_NAME) and PREDICATE instead of INDEX_ID, e.g. filter
WHERE TABLE_SCHEMA='test' AND TABLE_NAME='pairs20' AND INDEX_NAME IN
('<partial_idx_name1>','<partial_idx_name2>') AND PREDICATE = '`id` > 5' (or use
a single INDEX_NAME = '<name>' if only one), then adjust the expected RESCNT
accordingly so the check is deterministic.
|
@ti-chi-bot: 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. |
|
@ti-chi-bot: The following test 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. |
This is an automated cherry-pick of #64434
What problem does this PR solve?
Issue Number: ref #62664
Problem Summary:
To be compatible with partial index for repairing ingest index after log restore
What changed and how does it work?
recognize the
IndexInfo.ConditionExprStringCheck List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
Release Notes
New Features
Tests