Skip to content

Commit

Permalink
Merge pull request #117454 from cockroachdb/blathers/backport-release…
Browse files Browse the repository at this point in the history
…-23.2.0-rc-117433

release-23.2.0-rc: sql: do not re-run optbuild before collecting index recommendations
  • Loading branch information
michae2 authored Jan 8, 2024
2 parents 56ddbad + 7dbe1b8 commit 49175ee
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 11 deletions.
28 changes: 17 additions & 11 deletions pkg/sql/instrumentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1044,21 +1044,27 @@ func (ih *instrumentationHelper) SetIndexRecommendations(
recommendations = ih.explainIndexRecs
} else {
opc := &planner.optPlanningCtx
opc.reset(ctx)
f := opc.optimizer.Factory()
evalCtx := opc.p.EvalContext()
f.Init(ctx, evalCtx, opc.catalog)
f.FoldingControl().AllowStableFolds()
bld := optbuilder.New(ctx, &opc.p.semaCtx, evalCtx, opc.catalog, f, opc.p.stmt.AST)
err := bld.Build()
if err != nil {
log.Warningf(ctx, "unable to build memo: %s", err)
} else {
recommendations, err = opc.makeQueryIndexRecommendation(ctx)
if f.Memo() == nil || f.Memo().IsEmpty() {
// Run optbuild to create a memo with a root expression, if the current
// memo is empty.
opc.reset(ctx)
evalCtx := opc.p.EvalContext()
f.Init(ctx, evalCtx, opc.catalog)
f.FoldingControl().AllowStableFolds()
bld := optbuilder.New(ctx, &opc.p.semaCtx, evalCtx, opc.catalog, f, opc.p.stmt.AST)
err := bld.Build()
if err != nil {
log.Warningf(ctx, "unable to generate index recommendations: %s", err)
log.Warningf(ctx, "unable to build memo: %s", err)
return
}
}
var err error
recommendations, err = opc.makeQueryIndexRecommendation(ctx)
if err != nil {
log.Warningf(ctx, "unable to generate index recommendations: %s", err)
return
}
}
reset = true
}
Expand Down
107 changes: 107 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/index_recommendations
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# LogicTest: local

# Regression test for #117307.

statement ok
SET CLUSTER SETTING sql.metrics.statement_details.index_recommendation_collection.enabled = true

statement ok
SET enable_insert_fast_path = true

statement ok
CREATE TABLE b (b INT PRIMARY KEY)

statement ok
CREATE TABLE c (c INT PRIMARY KEY)

statement ok
CREATE TABLE d (d INT PRIMARY KEY)

statement ok
CREATE TABLE abcd (
a INT NOT NULL PRIMARY KEY,
b INT NULL REFERENCES b (b),
c INT NULL REFERENCES c (c),
d INT NULL REFERENCES d (d),
INDEX (b),
INDEX (c),
INDEX (d)
)

statement ok
INSERT INTO b VALUES (0), (1), (2), (3), (4), (5)

statement ok
INSERT INTO c VALUES (0), (1), (2), (3), (4), (5)

statement ok
INSERT INTO d VALUES (0), (1), (2), (3), (4)

# We need 5 previous executions to turn on index recommendation collection.

statement ok
PREPARE p0 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement ok
EXECUTE p0(0, 0, 0, 0)

statement ok
DEALLOCATE p0

statement ok
PREPARE p1 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement ok
EXECUTE p1(1, 1, 1, 1)

statement ok
DEALLOCATE p1

statement ok
PREPARE p2 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement ok
EXECUTE p2(2, 2, 2, 2)

statement ok
DEALLOCATE p2

statement ok
PREPARE p3 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement ok
EXECUTE p3(3, 3, 3, 3)

statement ok
DEALLOCATE p3

statement ok
PREPARE p4 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement ok
EXECUTE p4(4, 4, 4, 4)

statement ok
DEALLOCATE p4

# To hit the bug, we need a combination of:
# - index recommendation collection
# - prepared statement
# - insert fast path with multiple FK checks
# - one FK check can be skipped due to NULL value
# - last FK check fails

statement ok
PREPARE p5 AS INSERT INTO abcd VALUES ($1, $2, $3, $4)

statement error pq: insert on table "abcd" violates foreign key constraint "abcd_d_fkey"\nDETAIL: Key \(d\)=\(5\) is not present in table "d"
EXECUTE p5(5, 5, NULL, 5)

statement ok
DEALLOCATE p5

statement ok
RESET CLUSTER SETTING sql.metrics.statement_details.index_recommendation_collection.enabled

statement ok
RESET enable_insert_fast_path
7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/local/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/sql/schemachanger/comparator_generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 49175ee

Please sign in to comment.