Skip to content

Commit

Permalink
sql: set index recommendations after planning but before execution
Browse files Browse the repository at this point in the history
This commit moves the call to set the index recommendations to be done
right after planning was completed. Previously, it was done after the
execution, but it makes more sense to do it after planning. This also
allows us to remove the check on the txn still being open.

Epic: None

Release note: None
  • Loading branch information
yuzefovich committed Mar 20, 2023

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
1 parent cb58230 commit 1f299e2
Showing 1 changed file with 6 additions and 11 deletions.
17 changes: 6 additions & 11 deletions pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
@@ -1400,17 +1400,6 @@ func (ex *connExecutor) dispatchToExecutionEngine(

populateQueryLevelStatsAndRegions(ctx, planner, ex.server.cfg, &stats, &ex.cpuStatsCollector)

// The transaction (from planner.txn) may already have been committed at this point,
// due to one-phase commit optimization or an error. Since we use that transaction
// on the optimizer, check if is still open before generating index recommendations.
if planner.txn.IsOpen() {
// Set index recommendations, so it can be saved on statement statistics.
// TODO(yuzefovich): figure out whether we want to set isInternalPlanner
// to true for the internal executors.
isInternal := ex.executorType == executorTypeInternal || planner.isInternalPlanner
planner.instrumentation.SetIndexRecommendations(ctx, ex.server.idxRecommendationsCache, planner, isInternal)
}

// Record the statement summary. This also closes the plan if the
// plan has not been closed earlier.
stmtFingerprintID = ex.recordStatementSummary(
@@ -1709,6 +1698,12 @@ func (ex *connExecutor) makeExecPlan(ctx context.Context, planner *planner) erro
ex.extraTxnState.numDDL++
}

// Set index recommendations, so it can be saved on statement statistics.
// TODO(yuzefovich): figure out whether we want to set isInternalPlanner
// to true for the internal executors.
isInternal := ex.executorType == executorTypeInternal || planner.isInternalPlanner
planner.instrumentation.SetIndexRecommendations(ctx, ex.server.idxRecommendationsCache, planner, isInternal)

return nil
}

0 comments on commit 1f299e2

Please sign in to comment.