pingcap / pingcap/tidb

server, executor: PLAN REPLAYER DUMP EXPLAIN from SQL file can skip file transfer in the current statement

Open
#69,630 5 comments 0 reactions 0 assignees View on GitHub
contribution severity/minor sig/execution type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

Please answer these questions before submitting your issue. Thanks!

### 1. Minimal reproduce step (Required)

This issue is about the SQL-file form of plan replayer dump:

```sql
PLAN REPLAYER DUMP EXPLAIN 'sql.txt';
```

Current problem: this statement can set up `PlanReplayerDumpVarKey` and return through the result-set path without reading `sql.txt` or generating the plan replayer dump in the same statement. The pending dump state is left in the session and can then be consumed by a later unrelated no-result statement.

Potential user-level reproduction path:

1. Start TiDB and connect with a client that enables `LOCAL INFILE` / `CLIENT_LOCAL_FILES`.
2. Prepare a local SQL file, for example `sql.txt`, containing:

```sql
select 1;
```

3. Run:

```sql
PLAN REPLAYER DUMP EXPLAIN 'sql.txt';
```

4. Observe that the current statement can return through the `File_token` result-set path before the server-side SQL-file transfer handler runs. The dump file/token is therefore not produced by the statement that requested it.
5. Run an unrelated no-result statement in the same connection, for example:

```sql
SET @a = 1;
```

6. That later statement can unexpectedly consume the stale `PlanReplayerDumpVarKey` and read `sql.txt` as a side effect of the wrong statement.

Code-path evidence

- The parser supports the SQL-file form: `PLAN REPLAYER DUMP EXPLAIN stringLit` and `PLAN REPLAYER DUMP EXPLAIN ANALYZE stringLit`.
- `PlanBuilder.buildPlanReplayer` always sets a one-column result schema named `File_token`, so the statement follows the result-set execution path.
- In `PlanReplayerExec.Next`, the SQL-file case calls `prepare()`, stores `PlanReplayerDumpVarKey` in the session context, sets `endFlag`, and returns without appending a `File_token` row. The comment says the SQL-file dump is called in `handleFileTransInConn()`.
- In `server.clientConn.handleStmt`, when `rs != nil`, the server writes the result set and returns before reaching `handleFileTransInConn()`.
- `server.clientConn.handleFileTransInConn()` still contains the consumer for `PlanReplayerDumpVarKey`, but that function is only reached from the no-result path.

Relevant functions:

- `pkg/parser/parser.y`: `PLAN REPLAYER DUMP ... stringLit` grammar
- `pkg/planner/core/planbuilder.go`: `buildPlanReplayer`
- `pkg/executor/plan_replayer.go`: `PlanReplayerExec.Next`, `PlanReplayerExec.prepare`, `PlanReplayerDumpInfo.DumpSQLsFromFile`
- `pkg/server/conn.go`: `clientConn.handleStmt`, `clientConn.handlePlanReplayerDump`, `clientConn.handleFileTransInConn`

### 2. What did you expect to see? (Required)

`PLAN REPLAYER DUMP EXPLAIN 'sql.txt'` should handle the local SQL-file transfer, parse `sql.txt`, dump plan replayer information, clear `PlanReplayerDumpVarKey`, and return the `File_token` result as part of the same statement.

A later unrelated statement in the same connection should never observe or consume stale plan replayer dump state.

### 3. What did you see instead (Required)

The current code path can return from the result-set branch before consuming `PlanReplayerDumpVarKey`:

- The first `PLAN REPLAYER DUMP EXPLAIN 'sql.txt'` statement may return an empty/incorrect `File_token` result because `PlanReplayerExec.Next` stores the pending dump state but does not read the SQL file or append the final token row.
- `PlanReplayerDumpVarKey` can remain in the session after the statement finishes.
- A later unrelated no-result statement can then enter `handleFileTransInConn()` and perform the old plan replayer dump under the wrong statement context.

So the concrete bug is not just that the internal state is inconsistent. The user-visible operation requested by `PLAN REPLAYER DUMP EXPLAIN 'sql.txt'` is not completed by that statement, and stale state can leak into later statements on the same connection.

This is separate from #69587, which tracks `LOAD DATA` slow-log behavior caused by a different file-transfer marker mismatch.

### 4. What is your TiDB version? (Required)

Current `master` code path, observed by code inspection while working around PR #69552. No full end-to-end `SELECT tidb_version()` reproduction was captured locally yet.

Contributor guide

Open the contributing guide

Research direction

Trace the SQL-file path through pkg/executor/plan_replayer.go and pkg/server/conn.go, starting with PlanReplayerExec.Next and clientConn.handleStmt. Use the provided PLAN REPLAYER DUMP EXPLAIN 'sql.txt' followed by SET @a = 1 reproduction, then inspect the parser and planner references. Done means the first statement transfers and parses the file, returns its File_token, clears PlanReplayerDumpVarKey, and does not leak state to the later statement.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.