sql: clean up uses of Statement#56217
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 18 of 18 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @andreimatei, @nvanbenschoten, and @RaduBerinde)
pkg/sql/conn_executor_exec.go, line 203 at r1 (raw file):
// StatementType() different from "Rows". if portal.exhausted { return nil, nil, nil
nit: not sure if it's important, but maybe we should just have return without nils so that default ev, payload, err values are used?
andreimatei
left a comment
There was a problem hiding this comment.
💯
Reviewable status:
pkg/sql/conn_executor_exec.go, line 203 at r1 (raw file):
Previously, yuzefovich wrote…
nit: not sure if it's important, but maybe we should just have
returnwithoutnils so that defaultev, payload, errvalues are used?
no... no naked returns pls :)
ea6c185 to
dccfda9
Compare
|
bors r+ |
|
Build failed: |
This change cleans up the use of `sql.Statement` and reduces some allocations. Specifically: - we create a `Statement` lower in the stack (in execStmtInOpenState), and pass only what we need in the higher layers; - we change various functions to take a `tree.Statement` rather than an entire Statement when possible; - we move down the withStatement context allocation, so that it is avoided in the implicit transaction state transition; - we store a copy rather than a pointer to the Statement in the planner; - we avoid directly using `stmt` fields from `func()` declarations that escape; - we populate `Statement.AnonymizedStr` upfront. The anonymized string is always needed (to update statement stats). ``` name old time/op new time/op delta EndToEnd/kv-read/EndToEnd 153µs ± 1% 154µs ± 2% ~ (p=0.486 n=4+4) EndToEnd/kv-read-no-prep/EndToEnd 216µs ± 1% 217µs ± 1% ~ (p=0.886 n=4+4) EndToEnd/kv-read-const/EndToEnd 111µs ± 1% 113µs ± 1% +1.01% (p=0.029 n=4+4) name old alloc/op new alloc/op delta EndToEnd/kv-read/EndToEnd 25.8kB ± 1% 25.5kB ± 1% ~ (p=0.114 n=4+4) EndToEnd/kv-read-no-prep/EndToEnd 32.2kB ± 1% 31.9kB ± 1% ~ (p=0.686 n=4+4) EndToEnd/kv-read-const/EndToEnd 21.0kB ± 1% 20.7kB ± 2% ~ (p=0.200 n=4+4) name old allocs/op new allocs/op delta EndToEnd/kv-read/EndToEnd 252 ± 1% 250 ± 0% -0.99% (p=0.029 n=4+4) EndToEnd/kv-read-no-prep/EndToEnd 332 ± 0% 330 ± 1% ~ (p=0.229 n=4+4) EndToEnd/kv-read-const/EndToEnd 214 ± 0% 212 ± 0% -0.93% (p=0.029 n=4+4) ``` Release note: None
dccfda9 to
fb101b0
Compare
|
bors r+ |
|
Build succeeded: |
This change cleans up the use of
sql.Statementand reduces someallocations. Specifically:
we create a
Statementlower in the stack (inexecStmtInOpenState),and pass only what we need in the higher layers;
we change various functions to take a
tree.Statementrather thanan entire
Statementwhen possible;we move down the
withStatementcontext allocation, so that it isavoided in the implicit transaction state transition;
we store a copy rather than a pointer to the Statement in the
planner;
we avoid directly using
stmtfields fromfunc()declarationsthat escape;
we populate
Statement.AnonymizedStrupfront. The anonymizedstring is always needed (to update statement stats).
Release note: None