Skip to content

meta, planner: add the UnsignedFlag to the type of _tidb_commit_ts#66656

Merged
ti-chi-bot[bot] merged 3 commits intopingcap:masterfrom
time-and-fate:s29-aa-m-commit-ts-add-unsigned
Mar 3, 2026
Merged

meta, planner: add the UnsignedFlag to the type of _tidb_commit_ts#66656
ti-chi-bot[bot] merged 3 commits intopingcap:masterfrom
time-and-fate:s29-aa-m-commit-ts-add-unsigned

Conversation

@time-and-fate
Copy link
Copy Markdown
Member

@time-and-fate time-and-fate commented Mar 3, 2026

What problem does this PR solve?

Issue Number: ref #64281

What changed and how does it work?

As the title says.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

  • Chores
    • Marked an internal timestamp column as unsigned to improve compatibility and consistency when handling numeric values. This ensures the column is treated as a non-negative integer across schemas and validations, reducing potential mismatches when reading, validating, or comparing stored timestamps.

@ti-chi-bot ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label Mar 3, 2026
@pantheon-ai
Copy link
Copy Markdown

pantheon-ai bot commented Mar 3, 2026

Review Complete

Findings: 1 issues
Posted: 0
Duplicates/Skipped: 1

ℹ️ Learn more details on Pantheon AI.

@ti-chi-bot ti-chi-bot bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 3, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

Added the mysql.UnsignedFlag to the ExtraCommitTS column's flags in two places (meta model and planner type), changing the column to unsigned without other behavioral or control-flow changes.

Changes

Cohort / File(s) Summary
Meta column update
pkg/meta/model/column.go
In NewExtraCommitTSColInfo, set colInfo flags to include mysql.UnsignedFlag via bitwise OR; no other attributes changed.
Planner type update
pkg/planner/core/operator/logicalop/logical_datasource.go
Marked the ExtraCommitTS schema column's return type as unsigned by OR-ing mysql.UnsignedFlag onto the longlong RetType; no control-flow changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudged a flag, a quiet tweak,
A tiny hop — the types now speak,
Unsigned and neat, the timestamp gleams,
Small change, soft thump, and happy dreams.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. It lacks a problem summary, detailed explanation of what changed, and adequate context. The 'What changed and how does it work?' section only states 'As the title says.' without elaboration. Expand the 'Problem Summary' and 'What changed and how does it work?' sections with specific details about why UnsignedFlag is needed and how the changes address the issue.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately describes the main change: adding the UnsignedFlag to the _tidb_commit_ts column type across two files (meta and planner modules).

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

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

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/meta/model/column.go (1)

378-392: ⚠️ Potential issue | 🟠 Major

Add unit test for NewExtraCommitTSColInfo() in column_test.go following the existing pattern.

This new metadata column constructor needs test coverage. Add a test similar to the existing one for NewExtraPhysTblIDColInfo() at line 103-104 of pkg/meta/model/column_test.go to verify the ColumnInfo is initialized correctly with the expected type and flags (TypeLonglong + UnsignedFlag).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/meta/model/column.go` around lines 378 - 392, Add a unit test in
pkg/meta/model/column_test.go mirroring the existing test for
NewExtraPhysTblIDColInfo: call NewExtraCommitTSColInfo(), assert the returned
*ColumnInfo has ID == ExtraCommitTSID and Name == ExtraCommitTSName, and verify
its type and flags are set to mysql.TypeLonglong with mysql.UnsignedFlag (also
check flen/decimal/charset/collate if following the same assertions pattern used
by the NewExtraPhysTblIDColInfo test). Use the same test structure and helper
assertions as the existing test for NewExtraPhysTblIDColInfo to ensure
consistent coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pkg/meta/model/column.go`:
- Around line 378-392: Add a unit test in pkg/meta/model/column_test.go
mirroring the existing test for NewExtraPhysTblIDColInfo: call
NewExtraCommitTSColInfo(), assert the returned *ColumnInfo has ID ==
ExtraCommitTSID and Name == ExtraCommitTSName, and verify its type and flags are
set to mysql.TypeLonglong with mysql.UnsignedFlag (also check
flen/decimal/charset/collate if following the same assertions pattern used by
the NewExtraPhysTblIDColInfo test). Use the same test structure and helper
assertions as the existing test for NewExtraPhysTblIDColInfo to ensure
consistent coverage.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21b5bb6 and 4361094.

📒 Files selected for processing (1)
  • pkg/meta/model/column.go

@time-and-fate
Copy link
Copy Markdown
Member Author

/retest

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Mar 3, 2026
Copy link
Copy Markdown
Collaborator

@lcwangchao lcwangchao 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

Co-authored-by: Chao Wang <cclcwangchao@hotmail.com>
@pantheon-ai
Copy link
Copy Markdown

pantheon-ai bot commented Mar 3, 2026

Review Complete

Findings: 1 issues
Posted: 0
Duplicates/Skipped: 1

ℹ️ Learn more details on Pantheon AI.

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Mar 3, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Mar 3, 2026

[LGTM Timeline notifier]

Timeline:

  • 2026-03-03 09:57:56.254967382 +0000 UTC m=+265720.833046576: ☑️ agreed by xhebox.
  • 2026-03-03 10:04:27.688001905 +0000 UTC m=+266112.266081089: ☑️ agreed by lcwangchao.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.5729%. Comparing base (21b5bb6) to head (82c3962).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #66656        +/-   ##
================================================
- Coverage   77.6714%   77.5729%   -0.0986%     
================================================
  Files          2008       1929        -79     
  Lines        549230     537917     -11313     
================================================
- Hits         426595     417278      -9317     
+ Misses       120964     120621       -343     
+ Partials       1671         18      -1653     
Flag Coverage Δ
integration 41.3530% <100.0000%> (-6.8401%) ⬇️
unit 76.6465% <100.0000%> (+0.3286%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 56.7974% <ø> (ø)
parser ∅ <ø> (∅)
br 48.8331% <ø> (-12.0523%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@time-and-fate
Copy link
Copy Markdown
Member Author

/retest

1 similar comment
@time-and-fate
Copy link
Copy Markdown
Member Author

/retest

@lcwangchao
Copy link
Copy Markdown
Collaborator

/hold

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2026
@pantheon-ai
Copy link
Copy Markdown

pantheon-ai bot commented Mar 3, 2026

Review Complete

Findings: 0 issues
Posted: 0
Duplicates/Skipped: 0

ℹ️ Learn more details on Pantheon AI.

@ti-chi-bot ti-chi-bot bot added the sig/planner SIG: Planner label Mar 3, 2026
@time-and-fate time-and-fate changed the title meta: add the UnsignedFlag to the type of _tidb_commit_ts meta, planner: add the UnsignedFlag to the type of _tidb_commit_ts Mar 3, 2026
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot bot commented Mar 3, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lcwangchao, winoros, wjhuang2016, xhebox

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 approved label Mar 3, 2026
Copy link
Copy Markdown

@pantheon-ai pantheon-ai 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 looks good. No issues found.

@lcwangchao
Copy link
Copy Markdown
Collaborator

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 3, 2026
@lcwangchao
Copy link
Copy Markdown
Collaborator

/retest

@ti-chi-bot ti-chi-bot bot merged commit 0b23ce6 into pingcap:master Mar 3, 2026
32 checks passed
time-and-fate added a commit to time-and-fate/tidb that referenced this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants