sql,sqlstats: lift out fields required for logging from `dispatchToExecutionEngine`
sql: record topLevelQueryStats in instrumentationhelper
Previously topLevelQueryStats were being passed directly into
the logging function after execution in dispatchToExecutionEngine.
This commit persists the top level stats of the query execution
into the planner's instrumentation helper, similar to what we do
with sampled query exec stats. This allows us to move the logging
operation in dispatchToExecutionEngine to execStmtInOpenState.
Part of: https://github.com/cockroachdb/cockroach/issues/122722
Release note: None
sql, sqlstats: construct statement fingerprint id on demand when logging
Currently we construct the stmt fingerprint id at various places to pass to the logging function. We only need the statement fingerprint id for telemetry logging, so this construction is wasteful in other scenarios. This commit moves the stmt fingerprint id generation to the logging function.
Summary:
- Record the current stmt fingerprint id in the stats collector when writing sql stats. This value is reset at the start of stmt exec.
- Construct stmt fingerprint at the time of logging based on the AST in the planner if it was not available in the stats collector.
This change is motivated by the desire to move logging from
disatpchToExecutionEngine to execStmtInOpenState. See issue
for details.
Part of: https://github.com/cockroachdb/cockroach/issues/122722
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?
:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.
pkg/sql/conn_executor_exec.go line 622 at r1 (raw file):
Previously, abarganier (Alex Barganier) wrote…
Just confirming - the fingerprint will still be recomputed further down in the
maybeLogStatementcall for this scenario, right?
Yup, that's right.
TFTR! bors r=abarganier