diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index 6a569888fb48..c279c15deed2 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -1197,8 +1197,13 @@ func (ex *connExecutor) dispatchToExecutionEngine( populateQueryLevelStats(ctx, planner) - // Set index recommendations so it can be saved on statement statistics. - planner.instrumentation.SetIndexRecommendations(ctx, ex.server.idxRecommendationsCache, planner) + // 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. + planner.instrumentation.SetIndexRecommendations(ctx, ex.server.idxRecommendationsCache, planner) + } // Record the statement summary. This also closes the plan if the // plan has not been closed earlier. diff --git a/pkg/sql/idxrecommendations/idx_recommendations_cache_test.go b/pkg/sql/idxrecommendations/idx_recommendations_cache_test.go index f2565b0a855e..76ec199c9e5a 100644 --- a/pkg/sql/idxrecommendations/idx_recommendations_cache_test.go +++ b/pkg/sql/idxrecommendations/idx_recommendations_cache_test.go @@ -38,44 +38,63 @@ func TestIndexRecommendationsStats(t *testing.T) { testConn := sqlutils.MakeSQLRunner(sqlConn) testConn.Exec(t, "CREATE DATABASE idxrectest") testConn.Exec(t, "USE idxrectest") - testConn.Exec(t, "CREATE TABLE t ( k INT PRIMARY KEY, v INT, FAMILY \"primary\" (k, v))") - testConn.Exec(t, "CREATE TABLE t1 (k INT, i INT, f FLOAT, s STRING)") - testConn.Exec(t, "CREATE TABLE t2 (k INT, i INT, s STRING)") - testConn.Exec(t, "CREATE UNIQUE INDEX existing_t1_i ON t1(i)") + minExecutions := 5 + var recommendations string - testCases := []struct { - stmt string - fingerprint string - recommendations string - }{ - { - stmt: "SELECT * FROM t WHERE v > 123", - fingerprint: "SELECT * FROM t WHERE v > _", - recommendations: "{\"creation : CREATE INDEX ON t (v);\"}", - }, - { - stmt: "SELECT t1.k FROM t1 JOIN t2 ON t1.k = t2.k WHERE t1.i > 3 AND t2.i > 3", - fingerprint: "SELECT t1.k FROM t1 JOIN t2 ON t1.k = t2.k WHERE (t1.i > _) AND (t2.i > _)", - recommendations: "{\"replacement : CREATE UNIQUE INDEX ON t1 (i) STORING (k); DROP INDEX t1@existing_t1_i;\"," + - "\"creation : CREATE INDEX ON t2 (i) STORING (k);\"}", - }, - } + t.Run("index recommendations generated", func(t *testing.T) { + testConn.Exec(t, "CREATE TABLE t ( k INT PRIMARY KEY, v INT, FAMILY \"primary\" (k, v))") + testConn.Exec(t, "CREATE TABLE t1 (k INT, i INT, f FLOAT, s STRING)") + testConn.Exec(t, "CREATE TABLE t2 (k INT, i INT, s STRING)") + testConn.Exec(t, "CREATE UNIQUE INDEX existing_t1_i ON t1(i)") - var recommendations string - for i := 0; i < 8; i++ { - for _, tc := range testCases { - testConn.Exec(t, tc.stmt) + testCases := []struct { + stmt string + fingerprint string + recommendations string + }{ + { + stmt: "SELECT * FROM t WHERE v > 123", + fingerprint: "SELECT * FROM t WHERE v > _", + recommendations: "{\"creation : CREATE INDEX ON t (v);\"}", + }, + { + stmt: "SELECT t1.k FROM t1 JOIN t2 ON t1.k = t2.k WHERE t1.i > 3 AND t2.i > 3", + fingerprint: "SELECT t1.k FROM t1 JOIN t2 ON t1.k = t2.k WHERE (t1.i > _) AND (t2.i > _)", + recommendations: "{\"replacement : CREATE UNIQUE INDEX ON t1 (i) STORING (k); DROP INDEX t1@existing_t1_i;\"," + + "\"creation : CREATE INDEX ON t2 (i) STORING (k);\"}", + }, + } + + for i := 0; i < (minExecutions + 2); i++ { + for _, tc := range testCases { + testConn.Exec(t, tc.stmt) + rows := testConn.QueryRow(t, "SELECT index_recommendations FROM CRDB_INTERNAL.STATEMENT_STATISTICS "+ + " WHERE metadata ->> 'db' = 'idxrectest' AND metadata ->> 'query'=$1", tc.fingerprint) + rows.Scan(&recommendations) + + expected := tc.recommendations + if i < minExecutions { + expected = "{}" + } + require.Equal(t, expected, recommendations) + } + } + }) + + t.Run("index recommendations not generated for one-phase commit "+ + "optimization statement", func(t *testing.T) { + testConn.Exec(t, "CREATE TABLE t3 (k INT PRIMARY KEY)") + + for i := 0; i < (minExecutions + 2); i++ { + testConn.Exec(t, `INSERT INTO t3 VALUES($1)`, i) rows := testConn.QueryRow(t, "SELECT index_recommendations FROM CRDB_INTERNAL.STATEMENT_STATISTICS "+ - " WHERE metadata ->> 'db' = 'idxrectest' AND metadata ->> 'query'=$1", tc.fingerprint) + " WHERE metadata ->> 'db' = 'idxrectest' AND metadata ->> 'query' = 'INSERT INTO t3 VALUES ($1)'") rows.Scan(&recommendations) - expected := tc.recommendations - if i < 5 { - expected = "{}" - } - require.Equal(t, expected, recommendations) + require.Equal(t, "{}", recommendations) } - } + + }) } func TestFormatIdxRecommendations(t *testing.T) { diff --git a/pkg/sql/instrumentation.go b/pkg/sql/instrumentation.go index 60ec3283a675..a293758fb781 100644 --- a/pkg/sql/instrumentation.go +++ b/pkg/sql/instrumentation.go @@ -684,6 +684,8 @@ func (ih *instrumentationHelper) SetIndexRecommendations( opc.reset(ctx) stmtType := opc.p.stmt.AST.StatementType() + reset := false + var recommendations []string if idxRec.ShouldGenerateIndexRecommendation( ih.fingerprint, ih.planGist.Hash(), @@ -713,25 +715,17 @@ func (ih *instrumentationHelper) SetIndexRecommendations( err = opc.makeQueryIndexRecommendation(ctx) if err != nil { log.Warningf(ctx, "unable to generate index recommendations: %s", err) - } else { - idxRec.UpdateIndexRecommendations( - ih.fingerprint, - ih.planGist.Hash(), - planner.SessionData().Database, - stmtType, - ih.indexRecommendations, - true, - ) } } - } else { - ih.indexRecommendations = idxRec.UpdateIndexRecommendations( - ih.fingerprint, - ih.planGist.Hash(), - planner.SessionData().Database, - stmtType, - []string{}, - false, - ) + reset = true + recommendations = ih.indexRecommendations } + ih.indexRecommendations = idxRec.UpdateIndexRecommendations( + ih.fingerprint, + ih.planGist.Hash(), + planner.SessionData().Database, + stmtType, + recommendations, + reset, + ) }