From d272e1ccd43e0977f2ea8e85cd1ade9b8aa27b22 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Fri, 2 Dec 2022 20:23:43 +0000 Subject: [PATCH] sql: fix statement bundle creation when memo isn't detached 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 --- pkg/sql/explain_bundle.go | 8 +++++--- pkg/sql/explain_bundle_test.go | 13 +++++++++++++ pkg/sql/opt/norm/factory.go | 9 +++++++++ pkg/sql/plan_opt.go | 2 +- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/sql/explain_bundle.go b/pkg/sql/explain_bundle.go index c0bea873f755..341063b0a96f 100644 --- a/pkg/sql/explain_bundle.go +++ b/pkg/sql/explain_bundle.go @@ -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. // @@ -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 } diff --git a/pkg/sql/explain_bundle_test.go b/pkg/sql/explain_bundle_test.go index 5d95ae817fd0..eaa8e7c3372a 100644 --- a/pkg/sql/explain_bundle_test.go +++ b/pkg/sql/explain_bundle_test.go @@ -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) { diff --git a/pkg/sql/opt/norm/factory.go b/pkg/sql/opt/norm/factory.go index e2ff8d4eb5f6..7e5c27aabbec 100644 --- a/pkg/sql/opt/norm/factory.go +++ b/pkg/sql/opt/norm/factory.go @@ -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). diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go index 2a7a53631823..cfed3e879a27 100644 --- a/pkg/sql/plan_opt.go +++ b/pkg/sql/plan_opt.go @@ -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