Skip to content

Conversation

@hongyunyan
Copy link
Collaborator

@hongyunyan hongyunyan commented Feb 11, 2026

What problem does this PR solve?

Issue Number: close #3911

What is changed and how it works?

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`.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed schema comparison logic to properly account for index column prefix lengths. Schemas with different column prefix lengths are now correctly treated as distinct, improving cache behavior and schema handling accuracy.
  • Tests

    • Added test case for schema caching with different index column lengths.

@ti-chi-bot ti-chi-bot bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 11, 2026
@gemini-code-assist
Copy link

Summary of Changes

Hello @hongyunyan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of schema sharing by incorporating index column length into both the schema's unique identifier (digest) and its equality comparison logic. This change is crucial for preventing unintended schema sharing between tables that might otherwise appear similar but have different index prefix lengths, which could lead to incorrect primary key handle decoding, especially in scenarios involving clustered primary keys.

Highlights

  • Schema Hashing: The hashTableInfo function now includes the col.Length of index columns in its digest calculation. This ensures that tables with different index prefix lengths will generate distinct schema digests, preventing incorrect schema sharing.
  • Schema Equality Check: The sameColumnsAndIndices method has been updated to compare the col.Length of index columns. This makes schema equality checks stricter, preventing tables with differing index prefix lengths from being considered identical.
  • New Test Case: A new test, TestGetOrSetColumnSchema_DifferentIndexColumnLength, was added to validate that schemas with different index column lengths are correctly identified as distinct, confirming the enhanced strictness of schema management.

🧠 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
  • pkg/common/table_info_helper.go
    • Included index column length in the hashTableInfo digest calculation to ensure unique identification for schemas with varying index prefix lengths.
    • Added index column length comparison to sameColumnsAndIndices for stricter schema equality checks, preventing mismatches in handle decoding.
  • pkg/common/table_info_helper_test.go
    • Added TestGetOrSetColumnSchema_DifferentIndexColumnLength to verify that schemas with differing index column lengths are correctly treated as distinct, validating the new schema comparison logic.
Activity
  • No activity has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@ti-chi-bot ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 11, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 11, 2026

📝 Walkthrough

Walkthrough

The changes extend hashing and schema-equality logic in table info helpers to account for the length (prefix length) of index columns. This ensures schemas with different index column lengths are properly distinguished and not incorrectly shared.

Changes

Cohort / File(s) Summary
Index Column Length Handling
pkg/common/table_info_helper.go
Updated hashTableInfo to include column Length in hash calculations after Offset; modified sameColumnsAndIndices to require matching Length values for index columns when comparing schemas. Added clarifying comments on prefix length influence.
Test Coverage
pkg/common/table_info_helper_test.go
Added TestGetOrSetColumnSchema_DifferentIndexColumnLength to verify that schemas with identical base columns but different index column lengths are treated as distinct, resulting in separate cached schemas with different Digest values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

lgtm, approved

Suggested reviewers

  • wk989898
  • lidezhu

Poem

🐰 Hops with glee at lengths now weighed,
No more shall schemas be mislaid,
Each index column's prefix clear,
Correctness blooms throughout the year!

🚥 Pre-merge checks | ✅ 2 | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes the required issue reference but lacks details on what changed and how it works; most sections are left as empty template. Add a 'What is changed and how it works?' section explaining the index column length addition; answer the checklist questions about compatibility and documentation needs.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The PR implementation addresses part of issue #3911 by including index column length in hashing and equality checks, but does not fully address all stated objectives. Verify whether PKIsHandle, IsCommonHandle, and index state have also been added to the digest calculation as required by issue #3911, or clarify if the PR scope is intentionally narrower.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding index column length to schema hashing and equality checks to make schema sharing stricter.
Out of Scope Changes check ✅ Passed All changes relate to the primary objective of improving schema sharing correctness through enhanced hashing and equality checks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


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.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the strictness of schema sharing by incorporating the index column prefix length into the schema digest calculation and equality checks. The changes in pkg/common/table_info_helper.go correctly add col.Length to both hashTableInfo and sameColumnsAndIndices functions. This ensures that tables with different index prefix lengths are not considered to have the same schema, preventing potential issues with handle decoding. A new unit test, TestGetOrSetColumnSchema_DifferentIndexColumnLength, has been added to pkg/common/table_info_helper_test.go to verify this new behavior, confirming that different schemas and digests are generated as expected. The changes are correct and well-tested.

@ti-chi-bot
Copy link

ti-chi-bot bot commented Feb 11, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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 needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 11, 2026
@ti-chi-bot
Copy link

ti-chi-bot bot commented Feb 11, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-02-11 07:41:56.337304989 +0000 UTC m=+4269.417295684: ☑️ agreed by wk989898.

@ti-chi-bot ti-chi-bot bot added the approved label Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. 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.

Increase the correctness of schema sharing

2 participants