parser: support NDVRATE analyze syntax by 0xPoe · Pull Request #67608 · pingcap/tidb

0xPoe · 2026-04-08T07:40:11Z

What problem does this PR solve?

Issue Number: ref #67449

Problem Summary:
Add parser support for NDVRATE in ANALYZE TABLE ... WITH ... syntax.

What changed and how does it work?

Add NDVRATE as an analyze option token and AST enum.
Extend the analyze option grammar so ANALYZE TABLE t WITH 0.05 NDVRATE 0.00001 SAMPLERATE parses and restores canonically.
Regenerate parser artifacts and add parser tests.

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

Release note

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

None

Summary by CodeRabbit

New Features
- Added support for the NDVRATE option in ANALYZE TABLE ... WITH, allowing statements like WITH 0.05 NDVRATE and combinations such as WITH 0.05 NDVRATE, 0.00001 SAMPLERATE.
Tests
- Parser/restore tests updated to cover NDVRATE parsing and restoration.

pantheon-ai · 2026-04-08T07:40:18Z

@0xPoe I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details.

⏳ This process typically takes 10-30 minutes depending on the complexity of the changes.

_{ℹ️ Learn more details on Pantheon AI.}

coderabbitai · 2026-04-08T07:40:37Z

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: 70b53e9b-de89-40e4-a12e-0508bcecb139

📥 Commits

Reviewing files that changed from the base of the PR and between a9bd43c and 1b4aa53.

📒 Files selected for processing (7)

pkg/parser/ast/stats.go
pkg/parser/keywords.go
pkg/parser/keywords_test.go
pkg/parser/misc.go
pkg/parser/parser.go
pkg/parser/parser.y
pkg/parser/parser_test.go

✅ Files skipped from review due to trivial changes (4)

pkg/parser/misc.go
pkg/parser/keywords.go
pkg/parser/keywords_test.go
pkg/parser/parser_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/parser/parser.y

📝 Walkthrough

Walkthrough

Adds a new NDVRATE analyze option to the SQL parser: introduces a keyword token, maps it in the lexer, extends parser grammar to accept NDVRATE in ANALYZE TABLE ... WITH ..., updates AST enum/string, and adds parser tests.

Changes

Cohort / File(s)	Summary
AST / Analyze options `pkg/parser/ast/stats.go`	Added exported enum `AnalyzeOptNDVRate` and registered `"NDVRATE"` in `AnalyzeOptionString`.
Keyword registry & tests `pkg/parser/keywords.go`, `pkg/parser/keywords_test.go`	Added `NDVRATE` to `Keywords` and updated keywords length assertion (677 → 678).
Tokenizer mapping `pkg/parser/misc.go`	Mapped `"NDVRATE"` to token `ndvRate` in `tokenMap`.
Parser grammar & tests `pkg/parser/parser.y`, `pkg/parser/parser_test.go`	Added `NDVRATE` to TiDB keywords, extended `AnalyzeOptionList` to allow non-comma appends, and implemented rule parsing `NumLiteral "NDVRATE"`; added tests asserting parsing/restoration of `NDVRATE` cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

size/L, ok-to-test

Suggested reviewers

AilinKid
guo-shaoge

Poem

🐇 I hopped through tokens, soft and light,
NDVRATE nested snug and bright.
Numbers dance where parsers play,
Tables learned a new-fit way,
Hooray — the stats will now take flight! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding parser support for NDVRATE in the ANALYZE syntax, which aligns with the comprehensive changes across parser files.
Description check	✅ Passed	The description includes required sections: issue reference (ref `#67449`), problem summary, what changed and how, completed test checklist, affected areas, and release note. All mandatory information is present and well-documented.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

0xPoe · 2026-04-08T08:01:02Z

/retest

codecov · 2026-04-08T08:04:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.2706%. Comparing base (de09871) to head (1b4aa53).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67608        +/-   ##
================================================
+ Coverage   77.5839%   78.2706%   +0.6867%     
================================================
  Files          1981       1974         -7     
  Lines        547950     549836      +1886     
================================================
+ Hits         425121     430360      +5239     
+ Misses       122019     119040      -2979     
+ Partials        810        436       -374

Flag	Coverage Δ
integration	`43.7686% <ø> (+9.4289%)`	⬆️
unit	`76.8032% <ø> (+0.4699%)`	⬆️

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

Components	Coverage Δ
dumpling	`61.5065% <ø> (+0.0901%)`	⬆️
parser	`∅ <ø> (∅)`
br	`49.9100% <ø> (-10.5212%)`	⬇️

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

pkg/parser/misc.go (1)
729-729: Token map entry is out of alphabetical order.

The comment at line 157 says "Please try to keep the map in alphabetical order." The entry "NDVRATE" is currently placed after "S3", but it should be in the N-section (around lines 574-599, near entries like "NAMES", "NATIONAL", "NATURAL", etc.).
📝 Suggested placement

Move the entry from line 729 to the N-section, between "NTH_VALUE" and "NTILE" entries (or nearby based on exact alphabetical position). NDVRATE should come after "NCHAR" and before "NEVER" alphabetically.
 	"NCHAR":                         ncharType,
+	"NDVRATE":                        ndvRate,
 	"NEVER":                          never,
And remove from current location:
 	"S3":                             s3,
-	"NDVRATE":                        ndvRate,
 	"SAMPLES":                        samples,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/parser/misc.go` at line 729, The token map entry "NDVRATE": ndvRate is
out of alphabetical order; open the token map in pkg/parser/misc.go (the map
literal containing entries like "S3", "NAMES", "NATIONAL", "NTH_VALUE", "NTILE",
etc.), remove the "NDVRATE": ndvRate entry from its current S-section placement
and insert it into the N-section in the correct alphabetical position (e.g.,
after "NTH_VALUE"/"NCHAR" and before "NTILE"/"NEVER") so the entire map remains
alphabetically ordered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/parser/ast/stats.go`:
- Line 61: The new AnalyzeOptNDVRate token is added to the parser but not wired
into planning/execution; update the analyzeOptionLimit map to include
AnalyzeOptNDVRate with appropriate bounds, add a default value in
analyzeOptionDefaultV2 for NDVRATE, extend handleAnalyzeOptions to recognize and
validate NDVRATE (use the same validation flow as other numeric analyze
options), and modify the executor code that persists/loads statistics metadata
to store and retrieve the NDVRATE value so it flows from parser → planbuilder →
executor; if this PR intentionally only changes the parser, add an explicit
comment in the PR description and open a follow-up issue linking
AnalyzeOptNDVRate to the missing changes instead of implementing them now.

---

Nitpick comments:
In `@pkg/parser/misc.go`:
- Line 729: The token map entry "NDVRATE": ndvRate is out of alphabetical order;
open the token map in pkg/parser/misc.go (the map literal containing entries
like "S3", "NAMES", "NATIONAL", "NTH_VALUE", "NTILE", etc.), remove the
"NDVRATE": ndvRate entry from its current S-section placement and insert it into
the N-section in the correct alphabetical position (e.g., after
"NTH_VALUE"/"NCHAR" and before "NTILE"/"NEVER") so the entire map remains
alphabetically ordered.

🪄 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: c4a4ca6b-b461-4498-befc-9eb3adf094b7

📥 Commits

Reviewing files that changed from the base of the PR and between 078b070 and a9bd43c.

📒 Files selected for processing (7)

pkg/parser/ast/stats.go
pkg/parser/keywords.go
pkg/parser/keywords_test.go
pkg/parser/misc.go
pkg/parser/parser.go
pkg/parser/parser.y
pkg/parser/parser_test.go

0xPoe

🔢 Self-check (PR reviewed by myself and ready for feedback)

Code compiles successfully
Unit tests added
No AI-generated elegant nonsense in PR.
Bazel files updated
Comments added where necessary
PR title and description updated
Documentation PR created (or confirmed not needed)
PR size is reasonable

/cc @terry1purcell @henrybw

Copilot

Pull request overview

This PR extends TiDB’s SQL parser to recognize a new NDVRATE analyze option within ANALYZE TABLE ... WITH ..., enabling syntax like ANALYZE TABLE t WITH 0.05 NDVRATE 0.00001 SAMPLERATE and ensuring canonical restoration output.

Changes:

Added NDVRATE as a parser keyword/token and wired it into the lexer keyword map.
Extended the ANALYZE ... WITH option grammar and AST option enum/string mapping to support NDVRATE.
Added parser test cases covering NDVRATE alone and combined with SAMPLERATE.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/parser/parser.y	Adds `NDVRATE` token/keyword and grammar support for `NDVRATE` analyze options (including whitespace-separated option lists).
pkg/parser/parser_test.go	Adds parsing/restoration test coverage for `NDVRATE` analyze options.
pkg/parser/misc.go	Registers `NDVRATE` in the lexer token map.
pkg/parser/keywords.go	Adds `NDVRATE` to the keyword list (non-reserved, TiDB section).
pkg/parser/keywords_test.go	Updates expected keyword count after adding `NDVRATE`.
pkg/parser/ast/stats.go	Adds `AnalyzeOptNDVRate` and maps it to the `NDVRATE` restore string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ti-chi-bot · 2026-04-09T00:50:34Z

[LGTM Timeline notifier]

Timeline:

2026-04-08 17:29:45.847490566 +0000 UTC m=+977391.052850613: ☑️ agreed by terry1purcell.
2026-04-09 00:50:33.721266703 +0000 UTC m=+1003838.926626760: ☑️ agreed by henrybw.

Benjamin2037

LGTM

ti-chi-bot · 2026-04-10T14:17:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, henrybw, terry1purcell

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:

~~pkg/parser/OWNERS~~ [Benjamin2037,terry1purcell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

0xTars · 2026-04-10T14:21:44Z

/retest

0xTars · 2026-04-10T14:47:06Z

/retest

0xTars · 2026-04-10T15:07:23Z

/retest

0xTars · 2026-04-10T16:08:21Z

/retest

0xTars · 2026-04-10T16:13:26Z

/retest

0xTars · 2026-04-10T17:10:47Z

/retest

0xTars · 2026-04-10T17:15:51Z

/retest

0xTars · 2026-04-10T18:26:37Z

/retest

0xTars · 2026-04-10T19:02:00Z

/retest

0xTars · 2026-04-10T19:42:28Z

/retest

0xTars · 2026-04-11T03:41:52Z

/retest

0xTars · 2026-04-11T03:57:01Z

/retest

0xTars · 2026-04-11T04:37:23Z

/retest

0xTars · 2026-04-11T05:02:39Z

/retest

0xTars · 2026-04-11T05:07:43Z

/retest

0xTars · 2026-04-11T05:12:47Z

/retest

0xTars · 2026-04-11T05:43:04Z

/retest

0xTars · 2026-04-11T06:08:21Z

/retest

0xTars · 2026-04-11T10:20:39Z

/retest

ref pingcap#67449

ti-chi-bot Bot added the release-note-none Denotes a PR that doesn't merit a release note. label Apr 8, 2026

ti-chi-bot Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 8, 2026

coderabbitai Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread pkg/parser/ast/stats.go

0xPoe commented Apr 8, 2026

View reviewed changes

ti-chi-bot Bot requested review from henrybw and terry1purcell April 8, 2026 11:45

terry1purcell requested a review from Copilot April 8, 2026 17:28

terry1purcell approved these changes Apr 8, 2026

View reviewed changes

Copilot started reviewing on behalf of terry1purcell April 8, 2026 17:29 View session

ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 8, 2026

Copilot AI reviewed Apr 8, 2026

View reviewed changes

henrybw approved these changes Apr 9, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 9, 2026

0xPoe added 2 commits April 9, 2026 10:01

parser: support NDVRATE analyze syntax

a684b49

parser: revert unrelated parser formatting churn

1b4aa53

0xPoe force-pushed the parser-ndvrate-syntax branch from a9bd43c to 1b4aa53 Compare April 9, 2026 08:05

Benjamin2037 approved these changes Apr 10, 2026

View reviewed changes

ti-chi-bot Bot added the approved label Apr 10, 2026

ti-chi-bot Bot merged commit 40020bf into pingcap:master Apr 11, 2026
35 checks passed

premal pushed a commit to premal/tidb that referenced this pull request Apr 30, 2026

parser: support NDVRATE analyze syntax (pingcap#67608)

bb921c4

ref pingcap#67449

Conversation

0xPoe commented Apr 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

pantheon-ai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

0xPoe commented Apr 8, 2026

Uh oh!

codecov Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xPoe left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ti-chi-bot Bot commented Apr 9, 2026

[LGTM Timeline notifier]

Uh oh!

Benjamin2037 left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot Bot commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 10, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xTars commented Apr 11, 2026

Uh oh!

0xPoe commented Apr 8, 2026 •

edited by coderabbitai Bot

Loading

pantheon-ai Bot commented Apr 8, 2026 •

edited

Loading

coderabbitai Bot commented Apr 8, 2026 •

edited

Loading

codecov Bot commented Apr 8, 2026 •

edited

Loading