planner: make SELECT ... SKIP LOCKED fail loudly instead of silently not locking

[takaidohigasi]

What problem does this PR solve?

Issue Number: close #69720, close #61621, close #67715, ref #18207

Problem Summary:

SELECT ... FOR UPDATE SKIP LOCKED is parsed, but the lock type is unsupported everywhere downstream:

1. The statement silently degrades to a plain non-locking read (When passing SKIP LOCKED TiDB ignores FOR UPDATE #69720): two transactions can "lock" the same row at the same time, which silently breaks the FOR UPDATE guarantee.
2. On nonclustered-PK tables, planning can fail with Can't find column ..._tidb_rowid in schema (select for update skip locked failed with message Can't find column test.T._tidb_rowid in schema Column #61621, FOR UPDATE SKIP LOCKED on JOIN triggers _tidb_rowid resolution error in TiDB #67715): buildSelectLock still builds a LogicalLock carrying TblID2Handle, but LogicalLock.PruneColumns does not preserve the handle columns for unsupported lock types, so PhysicalLock.ResolveIndices cannot resolve them against the pruned child schema.

What changed and how does it work?

• Add a checkSelectLockInfo validation to preprocess (which also covers the PointGet fast path):
  • FOR UPDATE SKIP LOCKED now returns ErrNotSupportedYet (1235) This version of TiDB doesn't yet support 'SKIP LOCKED' instead of silently not locking.
  • FOR SHARE SKIP LOCKED follows the existing FOR SHARE noop handling: rejected unless noop functions are enabled, noop-with-warning when they are, and ErrNotSupportedYet when tidb_enable_shared_lock_promotion is enabled (promotion would execute it as FOR UPDATE).
• LogicalLock.PruneColumns now keeps the TblID2Handle/TblID2PhysTblIDCol columns from being pruned for unsupported (noop) lock types as well, since PhysicalLock is still built and resolves those columns against the child schema.

This is a preparatory, independently cherry-pickable step for the real SKIP LOCKED support tracked in #18207 (design doc and implementation in follow-up PRs).

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.

TestSelectLockSkipLocked in pkg/planner/core/preprocess_test.go covers the exact repro of #61621 (which fails with the _tidb_rowid error before this fix), the silent-no-op case of #69720, and the FOR SHARE noop/promotion matrix.

Side effects

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

Statements using FOR UPDATE SKIP LOCKED that previously (incorrectly) executed as plain reads now return error 1235. This behavior was a correctness bug; applications relying on it were not actually acquiring any locks.

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.

`SELECT ... FOR UPDATE SKIP LOCKED` now returns an `ErrNotSupportedYet` error instead of silently executing as a non-locking read, and no longer fails with a `_tidb_rowid` column resolution error on tables with a nonclustered primary key.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation for SKIP LOCKED clauses, including clearer handling of unsupported lock modes.
  • Ensured lock-related data remains available during query planning, including for tables without clustered primary keys.
  • Aligned FOR SHARE SKIP LOCKED behavior with shared-lock promotion and noop-function settings.
• Tests
  • Added coverage for supported and unsupported SKIP LOCKED scenarios and related error handling.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign king-dylan for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details

Needs approval from an approver in each of these files:

• pkg/planner/OWNERS

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]

Hi @takaidohigasi. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 16255624-89ef-4c7c-a6f5-a6dded49f78c

📥 Commits

Reviewing files that changed from the base of the PR and between b94006d and 52b7547.

📒 Files selected for processing (3)

• pkg/planner/core/operator/logicalop/logical_lock.go
• pkg/planner/core/preprocess.go
• pkg/planner/core/preprocess_test.go

---

📝 Walkthrough

Walkthrough

SKIP LOCKED validation now rejects unsupported lock modes, shared-lock noop handling includes SKIP LOCKED, and logical lock pruning preserves required handle and partition columns. Regression coverage exercises nonclustered primary-key queries and shared-lock promotion settings.

Changes

SKIP LOCKED handling

Layer / File(s)	Summary
Lock-clause validation and noop handling pkg/planner/core/preprocess.go	Select preprocessing validates FOR UPDATE SKIP LOCKED and FOR SHARE SKIP LOCKED, and applies shared-lock promotion rules to noop handling.
Lock column preservation and regression coverage pkg/planner/core/operator/logicalop/logical_lock.go, pkg/planner/core/preprocess_test.go	Logical lock pruning retains handle and partition physical table ID columns, with tests covering nonclustered primary keys, unsupported clauses, noop execution, and shared-lock promotion.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: cfzjywxk

Poem

A rabbit checks the lock in flight,
Rejects the paths that aren't just right.
Handles stay within the plan,
Rows avoid a missing span.
Hop, hop—tests confirm the byte!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: making SELECT ... SKIP LOCKED fail loudly instead of silently losing locking semantics.
Description check	✅ Passed	The description mostly matches the repository template and includes the issue line, problem summary, changes, checklist, side effects, and release note.
Linked Issues check	✅ Passed	The changes satisfy #69720, #61621, and #67715 by erroring on unsupported SKIP LOCKED and preserving lock columns for planning.
Out of Scope Changes check	✅ Passed	The diff appears focused on the SKIP LOCKED planner fix and its regression test, with no obvious unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

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)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: context deadline exceeded"
level=error msg="Timeout exceeded: try increasing it by passing --timeout option"

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}