planner: avoid the unconditional visitInfo copy in VisitInfo4PrivCheck
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Enhancement
`VisitInfo4PrivCheck` always allocates a new slice and copies every element, even when the
result is byte-for-byte identical to its input. Since column-level visit info was introduced
(#61741), `len(vs)` grew by an order of magnitude for wide `SELECT`s, so this copy went from
negligible to a measurable per-execution cost.
### Where
`pkg/planner/core/optimizer.go`, the `default` branch:
```go
default:
privVisitInfo = make([]visitInfo, 0, len(vs))
for _, v := range vs {
if needCheckTmpTablePriv(ctx, is, v) {
privVisitInfo = append(privVisitInfo, v)
}
}
```
`needCheckTmpTablePriv` returns `false` only for local temporary tables. On a cluster with no
local temporary tables — the common case — every element passes, and `privVisitInfo` ends up
equal to `vs`. The `make` is pure overhead. The `*ast.GrantStmt` branch right above already
returns the input slice directly (`privVisitInfo = vs`), so returning `vs` is an existing
pattern here.
This runs on every statement execution, including prepared-statement executes that hit the
plan cache, via `checkPreparedPriv` (`pkg/planner/core/plan_cache.go`).
### Measurement
Heap profile diff (`alloc_space`), v8.5.5 vs v8.5.6, identical 50s workload, 96 connections,
unistore. Query: `SELECT users.* FROM users JOIN auth ON users.id = auth.user_id WHERE auth.uuid = ?`
(`users` has 22 columns), run as a prepared statement.
| | v8.5.5 | v8.5.6 |
| --- | --- | --- |
| `VisitInfo4PrivCheck` flat | 204.04 MB / 955,158 objects | 2288.69 MB / 781,207 objects |
| bytes per call | **224 B** | **3,072 B** |
| implied `len(vs)` | 2 | ~25–27 |
`visitInfo` is 112 bytes, so 224 B is exactly two table-level entries; 3,072 B is the Go size
class for ~25–27 entries — the 22 `users` columns plus the `auth` columns and table-level
entries. One allocation per call, no hidden per-element allocation: `cap == len(vs)` means the
`append` loop never grows the slice.
Every one of those entries passed `needCheckTmpTablePriv`, so the copy produced an exact
duplicate of the input.
### Proposal
Allocate lazily, on the first element that actually needs to be dropped:
```go
default:
for i, v := range vs {
if needCheckTmpTablePriv(ctx, is, v) {
continue
}
privVisitInfo = make([]visitInfo, i, len(vs)-1)
copy(privVisitInfo, vs[:i])
for _, v := range vs[i+1:] {
if needCheckTmpTablePriv(ctx, is, v) {
privVisitInfo = append(privVisitInfo, v)
}
}
return
}
return vs
```
On clusters without local temporary tables this makes the function allocation-free.
The same shape applies to the `*ast.CreateTableStmt` and `*ast.DropTableStmt` branches — they
only diverge from the input when an entry is rewritten or dropped — but those are DDL paths
and not worth changing for allocation reasons.
### Not covered here
`needCheckTmpTablePriv` calls `is.TableByName` once per entry. With column-level visit info
that is ~27 lookups per statement where only 2 distinct `(db, table)` pairs are involved
(1162.14 MB vs 108.01 MB in the same profile pair). Deduplicating by `(db, table)` is a
separate change; the lazy allocation above does not address it.
Contributor guide
Research direction
Start in pkg/planner/core/optimizer.go at the default branch of VisitInfo4PrivCheck, then trace its prepared-statement path through checkPreparedPriv in pkg/planner/core/plan_cache.go. Preserve the existing filtering behavior while avoiding the copy when no entry is dropped, and verify the relevant planner tests and allocation behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100