From f02e9fc384a8ca5ec6caa38724b5fef1dbe0bcd0 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 6 Aug 2024 13:17:08 -0400 Subject: [PATCH 1/3] opt: remove documentation of old flags In #85935 we introduced the `set` flag to set session settings in optimizer tests and a few flags that were already controlling session settings were removed. The documentation of those old flags has been removed. Release note: None --- pkg/sql/opt/testutils/opttester/opt_tester.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pkg/sql/opt/testutils/opttester/opt_tester.go b/pkg/sql/opt/testutils/opttester/opt_tester.go index 08dd81b80d20..3e7d20a31b4d 100644 --- a/pkg/sql/opt/testutils/opttester/opt_tester.go +++ b/pkg/sql/opt/testutils/opttester/opt_tester.go @@ -502,12 +502,6 @@ func New(catalog cat.Catalog, sqlStr string) *OptTester { // - table: used to set the current table used by the command. This is used by // the inject-stats command. // -// - stats-quality-prefix: must be used with the stats-quality command. If -// rewriteActualFlag=true, indicates that a table should be created with the -// given prefix for the output of each subexpression in the query. Otherwise, -// outputs the name of the table that would be created for each -// subexpression. -// // - ignore-tables: specifies the set of stats tables for which stats quality // comparisons should not be outputted. Only used with the stats-quality // command. Note that tables can always be added to the `ignore-tables` set @@ -523,16 +517,6 @@ func New(catalog cat.Catalog, sqlStr string) *OptTester { // // - inject-stats: the file path is relative to the test file. // -// - join-limit: sets the value for SessionData.ReorderJoinsLimit, which -// indicates the number of joins at which the optimizer should stop -// attempting to reorder. -// -// - prefer-lookup-joins-for-fks: sets SessionData.PreferLookupJoinsForFKs to -// true, causing foreign key operations to prefer lookup joins. -// -// - null-ordered-last: sets SessionData.NullOrderedLast to true, which orders -// NULL values last in ascending order. -// // - cascade-levels: used to limit the depth of recursive cascades for // build-cascades. // From 6d761b200749e34e27577720019d28cde6bb7afe Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 6 Aug 2024 15:09:06 -0400 Subject: [PATCH 2/3] opt: disable `GenerateParameterizedJoin` for custom query plans The exploration rule `GenerateParameterizedJoin` is now disabled when a custom query plan is being built. The mechanism for toggling this rule is using either `Optimizer.Optimize()` which disables the rule or `Optimizer.OptimizeGeneric()` which enables the rule. Note that the rule is not toggled by the `plan_cache_mode` session setting, because the `auto` mode may build custom or generic query plans. Also, changing the `plan_cache_mode` session setting does not make memos stale as other session settings do, because generic memos are kept separate from the unoptimized memos reused to build custom plans. Release note: None --- pkg/sql/opt/testutils/opttester/opt_tester.go | 18 +++++- pkg/sql/opt/xform/optimizer.go | 22 ++++++++ pkg/sql/opt/xform/testdata/external/hibernate | 6 +- pkg/sql/opt/xform/testdata/external/nova | 16 +++--- pkg/sql/opt/xform/testdata/rules/generic | 56 ++++++++++++------- pkg/sql/plan_opt.go | 4 +- 6 files changed, 87 insertions(+), 35 deletions(-) diff --git a/pkg/sql/opt/testutils/opttester/opt_tester.go b/pkg/sql/opt/testutils/opttester/opt_tester.go index 3e7d20a31b4d..14c4935eb447 100644 --- a/pkg/sql/opt/testutils/opttester/opt_tester.go +++ b/pkg/sql/opt/testutils/opttester/opt_tester.go @@ -168,6 +168,9 @@ type Flags struct { // memo/check_expr.go. DisableCheckExpr bool + // Generic enables optimizations for generic query plans. + Generic bool + // ExploreTraceRule restricts the ExploreTrace output to only show the effects // of a specific rule. ExploreTraceRule opt.RuleName @@ -477,6 +480,8 @@ func New(catalog cat.Catalog, sqlStr string) *OptTester { // // - disable-check-expr: skips the assertions in memo/check_expr.go. // +// - generic: enables optimizations for generic query plans. +// // - rule: used with exploretrace; the value is the name of a rule. When // specified, the exploretrace output is filtered to only show expression // changes due to that specific rule. @@ -987,6 +992,9 @@ func (f *Flags) Set(arg datadriven.CmdArg) error { case "disable-check-expr": f.DisableCheckExpr = true + case "generic": + f.Generic = true + case "rule": if len(arg.Vals) != 1 { return fmt.Errorf("rule requires one argument") @@ -2323,15 +2331,19 @@ func (ot *OptTester) makeOptimizer() *xform.Optimizer { // not nil, allows the caller to update the table metadata before optimizing. func (ot *OptTester) optimizeExpr( o *xform.Optimizer, tables map[cat.StableID]cat.Table, -) (opt.Expr, error) { - err := ot.buildExpr(o.Factory()) +) (root opt.Expr, err error) { + err = ot.buildExpr(o.Factory()) if err != nil { return nil, err } if tables != nil { o.Memo().Metadata().UpdateTableMeta(ot.ctx, &ot.evalCtx, tables) } - root, err := o.Optimize() + if ot.Flags.Generic { + root, err = o.OptimizeGeneric() + } else { + root, err = o.Optimize() + } if err != nil { return nil, err } diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index 62b33ae40571..a5934948cb59 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -239,6 +239,28 @@ func (o *Optimizer) Memo() *memo.Memo { // equivalent to the given expression. If there is a cost "tie", then any one // of the qualifying lowest cost expressions may be selected by the optimizer. func (o *Optimizer) Optimize() (_ opt.Expr, err error) { + originalMatchedRule := o.matchedRule + defer func() { o.matchedRule = originalMatchedRule }() + o.matchedRule = func(r opt.RuleName) bool { + if r == opt.GenerateParameterizedJoin { + return false + } + if originalMatchedRule != nil { + // Respect the original callback, if one was set. + return originalMatchedRule(r) + } + return true + } + return o.optimize() +} + +// OptimizeGeneric is similar to Optimize. It enables all exploration rules +// designed specifically for optimization of generic query plans. +func (o *Optimizer) OptimizeGeneric() (_ opt.Expr, err error) { + return o.optimize() +} + +func (o *Optimizer) optimize() (_ opt.Expr, err error) { log.VEventf(o.ctx, 1, "optimize start") defer log.VEventf(o.ctx, 1, "optimize finish") defer func() { diff --git a/pkg/sql/opt/xform/testdata/external/hibernate b/pkg/sql/opt/xform/testdata/external/hibernate index 6a1d589183df..37ec682218cb 100644 --- a/pkg/sql/opt/xform/testdata/external/hibernate +++ b/pkg/sql/opt/xform/testdata/external/hibernate @@ -886,7 +886,7 @@ project └── filters └── min:14 = $1 [outer=(14), constraints=(/14: (/NULL - ]), fd=()-->(14)] -opt +opt generic select person0_.id as id1_2_, person0_.address as address2_2_, @@ -951,7 +951,7 @@ project └── filters └── max:16 = 0 [outer=(16), constraints=(/16: [/0 - /0]; tight), fd=()-->(16)] -opt +opt generic select person0_.id as id1_2_, person0_.address as address2_2_, @@ -1016,7 +1016,7 @@ project │ └── filters (true) └── filters (true) -opt +opt generic select person0_.id as id1_2_, person0_.address as address2_2_, diff --git a/pkg/sql/opt/xform/testdata/external/nova b/pkg/sql/opt/xform/testdata/external/nova index db8f578f49fb..a0d420805bcd 100644 --- a/pkg/sql/opt/xform/testdata/external/nova +++ b/pkg/sql/opt/xform/testdata/external/nova @@ -107,7 +107,7 @@ create table instance_type_extra_specs ) ---- -opt +opt generic select anon_1.flavors_created_at as anon_1_flavors_created_at, anon_1.flavors_updated_at as anon_1_flavors_updated_at, anon_1.flavors_id as anon_1_flavors_id, @@ -710,7 +710,7 @@ sort └── filters └── instance_type_extra_specs_1.instance_type_id:22 = instance_types.id:1 [outer=(1,22), constraints=(/1: (/NULL - ]; /22: (/NULL - ]), fd=(1)==(22), (22)==(1)] -opt +opt generic select anon_1.instance_types_created_at as anon_1_instance_types_created_at, anon_1.instance_types_updated_at as anon_1_instance_types_updated_at, anon_1.instance_types_deleted_at as anon_1_instance_types_deleted_at, @@ -861,7 +861,7 @@ project │ └── instance_type_extra_specs_1.deleted:37 = $7 [outer=(37), constraints=(/37: (/NULL - ]), fd=()-->(37)] └── filters (true) -opt +opt generic select anon_1.instance_types_created_at as anon_1_instance_types_created_at, anon_1.instance_types_updated_at as anon_1_instance_types_updated_at, anon_1.instance_types_deleted_at as anon_1_instance_types_deleted_at, @@ -1026,7 +1026,7 @@ project │ └── instance_type_extra_specs_1.deleted:37 = $7 [outer=(37), constraints=(/37: (/NULL - ]), fd=()-->(37)] └── filters (true) -opt +opt generic select anon_1.flavors_created_at as anon_1_flavors_created_at, anon_1.flavors_updated_at as anon_1_flavors_updated_at, anon_1.flavors_id as anon_1_flavors_id, @@ -1166,7 +1166,7 @@ project │ └── filters (true) └── filters (true) -opt +opt generic select anon_1.flavors_created_at as anon_1_flavors_created_at, anon_1.flavors_updated_at as anon_1_flavors_updated_at, anon_1.flavors_id as anon_1_flavors_id, @@ -1609,7 +1609,7 @@ sort └── filters └── instance_type_extra_specs_1.instance_type_id:36 = instance_types.id:1 [outer=(1,36), constraints=(/1: (/NULL - ]; /36: (/NULL - ]), fd=(1)==(36), (36)==(1)] -opt +opt generic select anon_1.instance_types_created_at as anon_1_instance_types_created_at, anon_1.instance_types_updated_at as anon_1_instance_types_updated_at, anon_1.instance_types_deleted_at as anon_1_instance_types_deleted_at, @@ -2357,7 +2357,7 @@ sort └── filters └── instance_type_extra_specs_1.instance_type_id:50 = instance_types.id:1 [outer=(1,50), constraints=(/1: (/NULL - ]; /50: (/NULL - ]), fd=(1)==(50), (50)==(1)] -opt +opt generic select anon_1.instance_types_created_at as anon_1_instance_types_created_at, anon_1.instance_types_updated_at as anon_1_instance_types_updated_at, anon_1.instance_types_deleted_at as anon_1_instance_types_deleted_at, @@ -2511,7 +2511,7 @@ project │ └── instance_type_extra_specs_1.deleted:37 = $7 [outer=(37), constraints=(/37: (/NULL - ]), fd=()-->(37)] └── filters (true) -opt +opt generic select anon_1.flavors_created_at as anon_1_flavors_created_at, anon_1.flavors_updated_at as anon_1_flavors_updated_at, anon_1.flavors_id as anon_1_flavors_id, diff --git a/pkg/sql/opt/xform/testdata/rules/generic b/pkg/sql/opt/xform/testdata/rules/generic index 93df29d979a5..fed232cac6d0 100644 --- a/pkg/sql/opt/xform/testdata/rules/generic +++ b/pkg/sql/opt/xform/testdata/rules/generic @@ -15,7 +15,7 @@ CREATE TABLE t ( # GenerateParameterizedJoin # -------------------------------------------------- -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE k = $1 ---- project @@ -42,7 +42,7 @@ project │ └── ($1,) └── filters (true) -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE k = $1::INT ---- project @@ -69,7 +69,7 @@ project │ └── ($1,) └── filters (true) -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND s = $2 AND b = $3 ---- project @@ -103,7 +103,7 @@ project # A placeholder referenced multiple times in the filters should only appear once # in the Values expression. -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE k = $1 AND i = $1 ---- project @@ -133,7 +133,7 @@ project # The generated join should not be reordered and merge joins should not be # explored on it. -opt expect=GenerateParameterizedJoin expect-not=(ReorderJoins,GenerateMergeJoins) +opt generic expect=GenerateParameterizedJoin expect-not=(ReorderJoins,GenerateMergeJoins) SELECT * FROM t WHERE i = $1 ---- project @@ -165,7 +165,7 @@ project │ └── filters (true) └── filters (true) -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE k = (SELECT i FROM t WHERE k = $1) ---- project @@ -209,7 +209,7 @@ project # TODO(mgartner): The rule doesn't apply because the filters do not reference # the placeholder directly. Consider ways to handle cases like this. -opt +opt generic SELECT * FROM t WHERE k = (SELECT $1::INT) ---- project @@ -242,7 +242,7 @@ exec-ddl CREATE INDEX partial_idx ON t(t) WHERE t IS NOT NULL ---- -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE t = $1 ---- project @@ -282,7 +282,7 @@ exec-ddl CREATE INDEX partial_idx ON t(i, t) WHERE i IS NOT NULL AND t IS NOT NULL ---- -opt expect=GenerateParameterizedJoin +opt generic expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND t = $2 ---- project @@ -322,7 +322,7 @@ exec-ddl CREATE INDEX partial_idx ON t(s) WHERE k = i ---- -opt +opt generic SELECT * FROM t@partial_idx WHERE s = $1 AND k = $2 AND i = $2 ---- project @@ -362,7 +362,7 @@ exec-ddl DROP INDEX partial_idx ---- -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE t = now() ---- project @@ -394,7 +394,7 @@ project │ └── filters (true) └── filters (true) -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND t = now() ---- project @@ -426,7 +426,7 @@ project │ └── filters (true) └── filters (true) -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND t > now() ---- project @@ -461,7 +461,7 @@ project │ └── filters (true) └── filters (true) -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND t = now() + $2 ---- project @@ -506,7 +506,7 @@ project │ └── filters (true) └── filters (true) -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND t = now() + '1 hr'::INTERVAL ---- project @@ -552,7 +552,7 @@ project └── filters (true) # TODO(mgartner): Apply the rule to stable, non-leaf expressions. -opt no-stable-folds +opt generic no-stable-folds SELECT * FROM t WHERE t = '2024-01-01 12:00:00'::TIMESTAMP::TIMESTAMPTZ ---- select @@ -571,7 +571,7 @@ select # arguments. # TODO(mgartner): We should be able to relax this restriction as long as all the # arguments are constants or placeholders. -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND s = quote_literal(1::INT) ---- project @@ -607,7 +607,7 @@ project # A stable function is not included in the Values expression if its arguments # reference a column from the table. This would create an illegal outer column # reference in a non-apply-join. -opt no-stable-folds expect=GenerateParameterizedJoin +opt generic no-stable-folds expect=GenerateParameterizedJoin SELECT * FROM t WHERE i = $1 AND s = quote_literal(i) ---- project @@ -642,7 +642,7 @@ project # The rule does not match if there are no placeholders or stable expressions in # the filters. -opt expect-not=GenerateParameterizedJoin +opt generic expect-not=GenerateParameterizedJoin SELECT * FROM t WHERE i = 1 AND s = 'foo' ---- index-join t @@ -654,3 +654,21 @@ index-join t ├── constraint: /2/3/4/1: [/1/'foo' - /1/'foo'] ├── key: (1) └── fd: ()-->(2,3), (1)-->(4) + +# The rule does not match if generic optimizations are disabled. +opt expect-not=GenerateParameterizedJoin +SELECT * FROM t WHERE k = $1 AND s = quote_literal(1::INT) +---- +select + ├── columns: k:1!null i:2 s:3!null b:4 t:5 + ├── cardinality: [0 - 1] + ├── has-placeholder + ├── key: () + ├── fd: ()-->(1-5) + ├── scan t + │ ├── columns: k:1!null i:2 s:3 b:4 t:5 + │ ├── key: (1) + │ └── fd: (1)-->(2-5) + └── filters + ├── k:1 = $1 [outer=(1), constraints=(/1: (/NULL - ]), fd=()-->(1)] + └── s:3 = e'\'1\'' [outer=(3), constraints=(/3: [/e'\'1\'' - /e'\'1\'']; tight), fd=()-->(3)] diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go index 8fc5b09753dd..b1f0d332e59c 100644 --- a/pkg/sql/plan_opt.go +++ b/pkg/sql/plan_opt.go @@ -515,7 +515,7 @@ func (opc *optPlanningCtx) buildReusableMemo( // Build a generic query plan if the placeholder fast path failed and a // generic plan was requested. opc.log(ctx, "optimizing (generic)") - if _, err := opc.optimizer.Optimize(); err != nil { + if _, err := opc.optimizer.OptimizeGeneric(); err != nil { return nil, memoTypeUnknown, err } opc.flags.Set(planFlagOptimized) @@ -1032,7 +1032,7 @@ func (opc *optPlanningCtx) makeQueryIndexRecommendation( // Save the normalized memo created by the optbuilder. savedMemo := opc.optimizer.DetachMemo(ctx) - // Use the optimizer to fully normalize the memo. We need to do this before + // Use the optimizer to fully optimize the memo. We need to do this before // finding index candidates because the *memo.SortExpr from the sort enforcer // is only added to the memo in this step. The sort expression is required to // determine certain index candidates. From 09563e12b52bdade7f99356aed31a8d071741cf1 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 6 Aug 2024 15:44:17 -0400 Subject: [PATCH 3/3] opt: add "Generic" optgen tag Optgen now supports the "Generic" tag and the `IsGeneric()` method has been added to `RuleName` which returns true if the rule has the tag. The new method is used to disable rules that optimize generic query plans when custom plans are being built. Release note: None --- .../opt/optgen/cmd/optgen/rule_names_gen.go | 35 +++++++++++++++++++ pkg/sql/opt/xform/optimizer.go | 2 +- pkg/sql/opt/xform/rules/generic.opt | 2 +- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go b/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go index 529333bf530c..c13c6e4a18ee 100644 --- a/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go +++ b/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go @@ -43,6 +43,9 @@ func (g *ruleNamesGen) generate(compiled *lang.CompiledExpr, w io.Writer) { fmt.Fprintf(g.w, " // NumRuleNames tracks the total count of rule names.\n") fmt.Fprintf(g.w, " NumRuleNames\n") fmt.Fprintf(g.w, ")\n\n") + + clear(g.unique) + g.genIsGeneric() } func (g *ruleNamesGen) genRuleNameEnumByTag(tag string) { @@ -64,3 +67,35 @@ func (g *ruleNamesGen) genRuleNameEnumByTag(tag string) { } fmt.Fprintf(g.w, "\n") } + +func (g *ruleNamesGen) genIsGeneric() { + fmt.Fprintf(g.w, "// IsGeneric returns true if r is tagged as a Generic rule.\n") + fmt.Fprintf(g.w, "func (r RuleName) IsGeneric() bool {\n") + fmt.Fprintf(g.w, " switch r {\n") + for _, rule := range g.compiled.Rules { + if !rule.Tags.Contains("Generic") { + continue + } + + // Only include each unique rule name once, not once per operator. + if _, ok := g.unique[rule.Name]; ok { + continue + } + g.unique[rule.Name] = struct{}{} + + if len(g.unique) == 1 { + fmt.Fprintf(g.w, " case ") + } else { + fmt.Fprintf(g.w, ", ") + } + fmt.Fprintf(g.w, "%s", rule.Name) + } + if len(g.unique) > 0 { + fmt.Fprintf(g.w, ":\n") + fmt.Fprintf(g.w, " return true\n") + } + fmt.Fprintf(g.w, " default:\n") + fmt.Fprintf(g.w, " return false\n") + fmt.Fprintf(g.w, " }\n") + fmt.Fprintf(g.w, "}\n\n") +} diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index a5934948cb59..cb270bb5ccb1 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -242,7 +242,7 @@ func (o *Optimizer) Optimize() (_ opt.Expr, err error) { originalMatchedRule := o.matchedRule defer func() { o.matchedRule = originalMatchedRule }() o.matchedRule = func(r opt.RuleName) bool { - if r == opt.GenerateParameterizedJoin { + if r.IsGeneric() { return false } if originalMatchedRule != nil { diff --git a/pkg/sql/opt/xform/rules/generic.opt b/pkg/sql/opt/xform/rules/generic.opt index c49631511e65..79996f777dc1 100644 --- a/pkg/sql/opt/xform/rules/generic.opt +++ b/pkg/sql/opt/xform/rules/generic.opt @@ -27,7 +27,7 @@ # | / \ | # Scan t Values ($1) Scan t Values ($1) # -[GenerateParameterizedJoin, Explore] +[GenerateParameterizedJoin, Explore, Generic] (Select $scan:(Scan $scanPrivate:*) & (IsCanonicalScan $scanPrivate) $filters:* &