executor: use statement context for pre-analyze stats flush

[0xPoe]

What problem does this PR solve?

Issue Number: close #68074

Problem Summary:
Pre-ANALYZE stats-delta flush used a detached context, so statement cancellation could be missed during executor build.

What changed and how does it work?

Pass the statement context into executorBuilder and use it for context-sensitive ANALYZE build work:

• pre-ANALYZE FLUSH STATS_DELTA during executor build;
• adaptive analyze DistSQL concurrency lookups in buildAnalyzeIndexPushdown, buildAnalyzeSamplingPushdown, and special-index NDV sub-jobs;
• auto sample-rate approximate table count lookup through getApproximateTableCountFromStorage.

Added regression coverage for canceled statement contexts during executor build.

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

• Bug Fixes
  • Improved context propagation so cancellations/deadlines are respected and analyze pre-flush avoids unnecessary work when canceled.
• Refactor
  • Internal separation of statement-scoped and session-scoped contexts to ensure session settings and long-running operations behave consistently.
• Tests
  • Added/updated tests and benchmarks to validate cancellation behavior and the new context separation during executor construction.

[pantheon-ai]

Review Complete

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

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

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Threads a statement-scoped context through executor construction, splitting executorBuilder into a statement context.Context (stmtCtx) and a session sessionctx.Context (sctx), makes pre-ANALYZE stats-delta flush cancellable via the statement context, and updates call sites and tests to pass the statement context.

Changes

Executor builder + analyze cancellation

Layer / File(s)	Summary
Data Shape / API pkg/executor/adapter.go	ExecStmt.buildExecutor() → ExecStmt.buildExecutor(ctx context.Context) and callers now pass the active statement ctx.
Core Builder pkg/executor/builder.go	executorBuilder now carries stmtCtx context.Context and sctx sessionctx.Context; newExecutorBuilder signature updated to accept (stmtCtx, sctx, ...). Many internal uses swapped from b.ctx to either b.stmtCtx (build-time ops) or b.sctx (session/txn/vars).
Pre-analyze flush pkg/executor/builder.go, pkg/executor/analyze.go	buildAnalyze asserts b.stmtCtx non-nil and flushes pending stats deltas using kv.WithInternalSourceType(b.stmtCtx, ...); flushStatsDeltaForAnalyze now checks ctx.Err() and returns early when cancelled.
Call-site updates / Wiring pkg/executor/point_get.go, pkg/executor/select.go, pkg/executor/coprocessor.go, pkg/executor/brie.go, pkg/executor/foreign_key.go, pkg/executor/index_merge_reader.go, ...	Numerous newExecutorBuilder invocations and executor constructions updated to pass (stmtCtx, sctx) and to read session variables / txn state from b.sctx rather than the former b.ctx.
Helpers / Tests / Bench pkg/executor/analyze_test.go, pkg/executor/analyze_utils_test.go, pkg/executor/benchmark_test.go, pkg/executor/builder_index_join_cleanup_test.go, pkg/executor/executor_required_rows_test.go, pkg/executor/traffic_test.go	Added BuildExecutorForTest(ctx, stmt *ExecStmt) error; new test TestBuildAnalyzePreFlushUsesStatementContext; test helpers/benchmarks updated to supply an explicit stmtCtx (often context.Background() in tests).

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client (user / KILL QUERY)
  participant Exec as ExecStmt
  participant Builder as ExecutorBuilder (stmtCtx, sctx)
  participant Stats as StatsFlush RPC
  Client->>Exec: run ANALYZE (with stmtCtx)
  Exec->>Builder: newExecutorBuilder(stmtCtx, sctx)
  Builder->>Stats: flushStatsDeltaForAnalyze(ctx = stmtCtx)
  alt ctx canceled before broadcast
    Stats-->>Builder: aborted (ctx.Err)
    Builder-->>Exec: return error (context.Canceled)
  else proceed
    Stats-->>Builder: broadcast/flush complete
    Builder-->>Exec: continue building analyzers
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• executor: fix analyze cannot be killed #65249: Threads caller context through analyze execution paths; closely related at the code/signature level.
• executor, statistics: flush pending stats delta before analyze #67939: Introduced pre-analyze flush using background context; this PR makes that flush cancellable and threads stmt ctx.

Suggested labels

sig/planner, component/statistics

Suggested reviewers

• henrybw
• qw4990
• mjonss
• terry1purcell

Poem

🐰
I carried ctx down root and leaf,
So flushes wake when queries grieve,
No background nap to block the way,
A canceled hop clears out the play,
ANALYZE now listens — nibble, cleave!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: passing statement context into executor building for the pre-ANALYZE stats flush operation.
Linked Issues check	✅ Passed	Changes comprehensively address issue #68074 by refactoring executorBuilder to carry statement context (stmtCtx) and using it for pre-ANALYZE stats flush, making it cancellable via KILL QUERY.
Out of Scope Changes check	✅ Passed	All changes are within scope: refactoring executorBuilder signature, updating all call sites with context arguments, and adding test coverage for canceled statement context during executor build.
Description check	✅ Passed	The PR description provides a clear issue reference, problem summary, and detailed explanation of changes with the specific contexts being passed through the executor builder.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

pkg/executor/adapter.go (2)

1399-1414: ⚡ Quick win

Document that ctx is the statement-scoped cancellation context.

Now that buildExecutor takes both ctx and sctx, the critical invariant is easy to miss. A short comment here would make it harder to reintroduce a detached background context in future call sites.

Suggested comment update

-// buildExecutor build an executor from plan, prepared statement may need additional procedure.
+// buildExecutor builds an executor from plan using the statement-scoped ctx for
+// cancellation-sensitive build steps (for example ANALYZE pre-work). Prepared
+// statements may need additional build steps.
 func (a *ExecStmt) buildExecutor(ctx context.Context) (exec.Executor, error) {

As per coding guidelines, "Comments SHOULD explain non-obvious intent, constraints, invariants, concurrency guarantees, SQL/compatibility contracts, or important performance trade-offs, and SHOULD NOT restate what the code already makes clear."

🤖 Prompt for AI Agents

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

In `@pkg/executor/adapter.go` around lines 1399 - 1414, Add a short comment at the
top of the buildExecutor function clarifying that the ctx parameter is the
statement-scoped cancellation context (tied to this statement's lifetime) and
must not be replaced with a background or long-lived context; reference the
function name buildExecutor and the local variable sctx to make the invariant
explicit so future changes won't detach cancellation from the session/statement.
Ensure the comment sits immediately above the function signature or the first
lines of the body where ctx and sctx are used.

---

681-684: 🏗️ Heavy lift

Please add or point to an integration test for the real KILL QUERY path.

The referenced unit coverage proves a pre-canceled build context fails fast, but the regression here was a user-visible ANALYZE cancellation path. An end-to-end test that interrupts ANALYZE through the actual kill/cancel wiring would protect that contract better.

Please confirm an existing integration test already covers this path, or add one if it does not.

Based on learnings, "For SQL behavior changes in executor, perform targeted unit test plus relevant integration test."

Also applies to: 1373-1375

🤖 Prompt for AI Agents

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

In `@pkg/executor/adapter.go` around lines 681 - 684, The change needs an
integration test that exercises the real KILL QUERY cancellation path for
ANALYZE rather than only the pre-canceled build context unit test; add (or point
to) an end-to-end test that starts an ANALYZE query, sends a KILL QUERY for that
session/statement and asserts the executor returns the expected cancellation
error/behavior (exercise the same codepath that calls buildExecutor and the
cancellation handling around lines referencing buildExecutor and the ANALYZE
cancel logic), or explicitly confirm an existing integration test already does
this; ensure the test targets the executor path that builds the executor
(buildExecutor) and the SQL-level kill wiring so the regression is caught.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@pkg/executor/adapter.go`:
- Around line 1399-1414: Add a short comment at the top of the buildExecutor
function clarifying that the ctx parameter is the statement-scoped cancellation
context (tied to this statement's lifetime) and must not be replaced with a
background or long-lived context; reference the function name buildExecutor and
the local variable sctx to make the invariant explicit so future changes won't
detach cancellation from the session/statement. Ensure the comment sits
immediately above the function signature or the first lines of the body where
ctx and sctx are used.
- Around line 681-684: The change needs an integration test that exercises the
real KILL QUERY cancellation path for ANALYZE rather than only the pre-canceled
build context unit test; add (or point to) an end-to-end test that starts an
ANALYZE query, sends a KILL QUERY for that session/statement and asserts the
executor returns the expected cancellation error/behavior (exercise the same
codepath that calls buildExecutor and the cancellation handling around lines
referencing buildExecutor and the ANALYZE cancel logic), or explicitly confirm
an existing integration test already does this; ensure the test targets the
executor path that builds the executor (buildExecutor) and the SQL-level kill
wiring so the regression is caught.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e4b1d00a-fab5-4794-adba-2e00bbfe784a

📥 Commits

Reviewing files that changed from the base of the PR and between 9d1ac81 and 8e3fc46.

📒 Files selected for processing (10)

• pkg/executor/adapter.go
• pkg/executor/analyze.go
• pkg/executor/analyze_test.go
• pkg/executor/analyze_utils_test.go
• pkg/executor/benchmark_test.go
• pkg/executor/builder.go
• pkg/executor/builder_index_join_cleanup_test.go
• pkg/executor/coprocessor.go
• pkg/executor/executor_required_rows_test.go
• pkg/executor/select.go

[pantheon-ai]

✅ Code looks good. No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 76.58863% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.0600%. Comparing base (9d1ac81) to head (f55a870).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #68146        +/-   ##
================================================
- Coverage   77.7620%   77.0600%   -0.7021%     
================================================
  Files          1990       1972        -18     
  Lines        551774     552612       +838     
================================================
- Hits         429071     425843      -3228     
- Misses       121783     126756      +4973     
+ Partials        920         13       -907     

Flag	Coverage Δ
integration	41.3647% <76.5886%> (+1.5628%)	⬆️

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

Components	Coverage Δ
dumpling	60.4888% <ø> (ø)
parser	∅ <ø> (∅)
br	50.0597% <ø> (-13.0338%)	⬇️

🚀 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: 2

🧹 Nitpick comments (1)

pkg/executor/builder.go (1)

3319-3324: ⚡ Quick win

Please add a relevant integration test for the SQL-level cancellation path.

The regression test described in the PR covers a pre-canceled statement context before executor build, but this is still an executor-layer SQL behavior change. It should also be exercised end-to-end with a real ANALYZE interrupted via statement cancel / KILL QUERY.

Based on learnings: "Applies to pkg/executor/** : For SQL behavior changes in executor, perform targeted unit test plus relevant integration test."

🤖 Prompt for AI Agents

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

In `@pkg/executor/builder.go` around lines 3319 - 3324, Add an end-to-end
integration test that exercises the SQL-level cancellation path for ANALYZE:
simulate a running ANALYZE statement that enters the executor path where
buildAnalyzeSamplingPushdown calls flushStatsDeltaForAnalyze (use a real TiDB
server test harness or cluster test that issues ANALYZE on a table), then cancel
it via statement cancel / KILL QUERY and assert that the server handles the
cancellation cleanly (no double-counting or leaked state) and that the error
path in flushStatsDeltaForAnalyze is exercised; locate the executor behavior
around buildAnalyzeSamplingPushdown, b.stmtCtx and kv.WithInternalSourceType to
ensure the test actually cancels while flushStatsDeltaForAnalyze is running.

🤖 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/executor/builder.go`:
- Around line 3300-3301: The call in getApproximateTableCountFromStorage
currently passes context.Background() to
pdhelper.GlobalPDHelper.GetApproximateTableCountFromStorage which ignores
statement cancellation; change it to use the builder's statement-scoped context
(use b.sctx's statement context) instead of context.Background() so ANALYZE
cancellations/KILL QUERY are respected—mirror the approach used by
flushStatsDeltaForAnalyze and pass the statement-scoped context from b.sctx into
GetApproximateTableCountFromStorage.

In `@pkg/executor/index_merge_reader.go`:
- Around line 194-195: The call to distsql.IndexRangesToKVRanges currently
ignores its error and proceeds to use keyRange; change this to capture the
returned error (e.g. kr, err := distsql.IndexRangesToKVRanges(...)) and handle
it instead of discarding it—either propagate the error up from the enclosing
method, or log and return an actionable error with context including
e.table.Meta().ID, idx.ID and which range (e.ranges[i]) failed; only assign to
e.partitionKeyRanges[i] when err is nil and keyRange is valid.

---

Nitpick comments:
In `@pkg/executor/builder.go`:
- Around line 3319-3324: Add an end-to-end integration test that exercises the
SQL-level cancellation path for ANALYZE: simulate a running ANALYZE statement
that enters the executor path where buildAnalyzeSamplingPushdown calls
flushStatsDeltaForAnalyze (use a real TiDB server test harness or cluster test
that issues ANALYZE on a table), then cancel it via statement cancel / KILL
QUERY and assert that the server handles the cancellation cleanly (no
double-counting or leaked state) and that the error path in
flushStatsDeltaForAnalyze is exercised; locate the executor behavior around
buildAnalyzeSamplingPushdown, b.stmtCtx and kv.WithInternalSourceType to ensure
the test actually cancels while flushStatsDeltaForAnalyze is running.

🪄 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: 874023d4-ce44-4d07-9e23-af07747a74a1

📥 Commits

Reviewing files that changed from the base of the PR and between 8e3fc46 and 4de2f97.

📒 Files selected for processing (7)

• pkg/executor/adapter.go
• pkg/executor/brie.go
• pkg/executor/builder.go
• pkg/executor/foreign_key.go
• pkg/executor/index_merge_reader.go
• pkg/executor/point_get.go
• pkg/executor/traffic_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 @mjonss

[0xPoe]

/retest

[mjonss]

LGTM, but I would like a different, not as overloaded, name for stmtCtx. Maybe simply 'ctx'?

[mjonss]

LGTM

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

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

• OWNERS [mjonss,terry1purcell]
• pkg/executor/OWNERS [mjonss,terry1purcell]

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]

[LGTM Timeline notifier]

Timeline:

• 2026-05-04 09:03:13.608349107 +0000 UTC m=+85666.481699089: ☑️ agreed by mjonss.
• 2026-05-04 13:57:43.77676322 +0000 UTC m=+103336.650113191: ☑️ agreed by terry1purcell.