Skip to content

Commit

Permalink
sql: fix statement bundle creation when memo isn't detached
Browse files Browse the repository at this point in the history
When a memo is deemed not resuable we don't detach it from the factory
and this causes problems later when we execute SetIndexRecommendations
which resets the optimizer context which will reset the memo.  This
causes the schema.sql and opt*.txt files to be empty.

Fixes: #92920

Release note: None
  • Loading branch information
cucaroach committed Dec 2, 2022
1 parent adf24a5 commit d272e1c
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 4 deletions.
8 changes: 5 additions & 3 deletions pkg/sql/explain_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import (
"github.com/cockroachdb/errors"
)

const noPlan = "no plan"

// setExplainBundleResult sets the result of an EXPLAIN ANALYZE (DEBUG)
// statement. warnings will be printed out as is in the CLI.
//
Expand Down Expand Up @@ -242,9 +244,9 @@ func (b *stmtBundleBuilder) addStatement() {
func (b *stmtBundleBuilder) addOptPlans() {
if b.plan.mem == nil || b.plan.mem.RootExpr() == nil {
// No optimizer plans; an error must have occurred during planning.
b.z.AddFile("opt.txt", "no plan")
b.z.AddFile("opt-v.txt", "no plan")
b.z.AddFile("opt-vv.txt", "no plan")
b.z.AddFile("opt.txt", noPlan)
b.z.AddFile("opt-v.txt", noPlan)
b.z.AddFile("opt-vv.txt", noPlan)
return
}

Expand Down
13 changes: 13 additions & 0 deletions pkg/sql/explain_bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ CREATE TABLE s.a (a INT PRIMARY KEY);`)
checkBundle(t, fmt.Sprintf("%+v", pqErr.Detail), "", nil, base, plans, "distsql.html")
})

// #92920 Make sure schema and opt files are created.
t.Run("memo-reset", func(t *testing.T) {
rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) CREATE TABLE t (i int)")
checkBundle(t, fmt.Sprint(rows), "", func(name, contents string) error {
if name == "opt.txt" {
if contents == noPlan {
return errors.Errorf("opt.txt empty")
}
}
return nil
}, base, plans, "distsql.html vec.txt vec-v.txt")
})

// Verify that we can issue the statement with prepare (which can happen
// depending on the client).
t.Run("prepare", func(t *testing.T) {
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/opt/norm/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ func (f *Factory) DetachMemo() *memo.Memo {
return m
}

// ReleaseMemo is just like DetachMemo but it doesn't call Detach on the memo
// preserving any statistics information for explain purposes (distinct, null
// count etc).
func (f *Factory) ReleaseMemo() *memo.Memo {
m := f.mem
f.mem = nil
return m
}

// DisableOptimizations disables all transformation rules. The unaltered input
// expression tree becomes the output expression tree (because no transforms
// are applied).
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/plan_opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ func (opc *optPlanningCtx) buildExecMemo(ctx context.Context) (_ *memo.Memo, _ e
return memo, nil
}

return f.Memo(), nil
return f.ReleaseMemo(), nil
}

// runExecBuilder execbuilds a plan using the given factory and stores the
Expand Down

0 comments on commit d272e1c

Please sign in to comment.