Merge #27126 #27132

27126: opt: improve the appliedRule callback r=RaduBerinde a=RaduBerinde #### opt: allow distinguishing between norm/explore rules Release note: None #### opt: improve the appliedRule callback Improving the appliedRule callback to also pass the ordinal of the expression that the rule was applied on. Cleaning up the related logic and state in forcing_opt. Release note: None 27132: ui: make sure we identify the cluster r=couchand a=couchand In #24996 I accidentally moved the identify call rather than copying it, meaning we frequently failed to issue the identify. This fixes that by trying to identify on every page if we have not yet. Release note: None Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Andrew Couch <[email protected]>
cockroachdb · Jul 3, 2018 · dfbd39b · dfbd39b
3 parents c071d5a + 130c9df + 445b2fc
commit dfbd39b
Show file tree

Hide file tree

Showing 14 changed files with 120 additions and 64 deletions.
diff --git a/pkg/sql/opt/memo/expr.go b/pkg/sql/opt/memo/expr.go
@@ -41,6 +41,9 @@ type ExprID struct {
 // expression.
 var InvalidExprID = ExprID{}
 
+// Suppress linter complaint.
+var _ = InvalidExprID
+
 // MakeNormExprID returns the id of the normalized expression for the given
 // group.
 func MakeNormExprID(group GroupID) ExprID {

diff --git a/pkg/sql/opt/norm/factory.go b/pkg/sql/opt/norm/factory.go
@@ -31,11 +31,15 @@ type MatchedRuleFunc func(ruleName opt.RuleName) bool
 
 // AppliedRuleFunc defines the callback function for the NotifyOnAppliedRule
 // event supported by the optimizer and factory. It is invoked each time an
-// optimization rule (Normalize or Explore) has been applied. The function is
-// called with the name of the rule and the memo group it affected. If the rule
-// was an exploration rule, then the added parameter gives the number of
-// expressions added to the group by the rule.
-type AppliedRuleFunc func(ruleName opt.RuleName, group memo.GroupID, added int)
+// optimization rule (Normalize or Explore) has been applied.
+//
+// The function is called with the name of the rule and the memo group it
+// affected. If the rule was an exploration rule, then the expr parameter
+// indicates the expression on which the rule was applied, and the added
+// parameter number of expressions added to the group by the rule.
+type AppliedRuleFunc func(
+	ruleName opt.RuleName, group memo.GroupID, expr memo.ExprOrdinal, added int,
+)
 
 // Factory constructs a normalized expression tree within the memo. As each
 // kind of expression is constructed by the factory, it transitively runs

diff --git a/pkg/sql/opt/optgen/cmd/optgen/rule_gen.go b/pkg/sql/opt/optgen/cmd/optgen/rule_gen.go
@@ -609,7 +609,7 @@ func (g *ruleGen) genNormalizeReplace(define *lang.DefineExpr, rule *lang.RuleEx
 
 	// Notify listeners that rule was applied.
 	g.w.nestIndent("if _f.appliedRule != nil {\n")
-	g.w.writeIndent("_f.appliedRule(opt.%s, _group, 0)\n", rule.Name)
+	g.w.writeIndent("_f.appliedRule(opt.%s, _group, 0, 0)\n", rule.Name)
 	g.w.unnest("}\n")
 
 	g.w.writeIndent("return _group\n")
@@ -657,7 +657,7 @@ func (g *ruleGen) genExploreReplace(define *lang.DefineExpr, rule *lang.RuleExpr
 	// Notify listeners that rule was applied.
 	g.w.nestIndent("if _e.o.appliedRule != nil {\n")
 	g.w.writeIndent("_after := _e.mem.ExprCount(_root.Group)\n")
-	g.w.writeIndent("_e.o.appliedRule(opt.%s, _root.Group, _after-_before)\n", rule.Name)
+	g.w.writeIndent("_e.o.appliedRule(opt.%s, _root.Group, _root.Expr, _after-_before)\n", rule.Name)
 	g.w.unnest("}\n")
 
 	g.w.unnest("}\n")

diff --git a/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go b/pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go
@@ -39,6 +39,9 @@ func (g *ruleNamesGen) generate(compiled *lang.CompiledExpr, w io.Writer) {
 	fmt.Fprintf(g.w, "  startAutoRule RuleName = iota + NumManualRuleNames\n\n")
 
 	g.genRuleNameEnumByTag("Normalize")
+	fmt.Fprintf(g.w, "  // startExploreRule tracks the number of normalization rules;\n")
+	fmt.Fprintf(g.w, "  // all rules greater than this value are exploration rules.\n")
+	fmt.Fprintf(g.w, "  startExploreRule\n\n")
 	g.genRuleNameEnumByTag("Explore")
 
 	fmt.Fprintf(g.w, "  // NumRuleNames tracks the total count of rule names.\n")

diff --git a/pkg/sql/opt/optgen/cmd/optgen/testdata/explorer b/pkg/sql/opt/optgen/cmd/optgen/testdata/explorer
@@ -178,7 +178,7 @@ func (_e *explorer) exploreInnerJoin(_rootState *exploreState, _root memo.ExprID
 				_e.mem.MemoizeDenormExpr(_root.Group, memo.Expr(_expr))
 				if _e.o.appliedRule != nil {
 					_after := _e.mem.ExprCount(_root.Group)
-					_e.o.appliedRule(opt.CommuteJoin, _root.Group, _after-_before)
+					_e.o.appliedRule(opt.CommuteJoin, _root.Group, _root.Expr, _after-_before)
 				}
 			}
 		}
@@ -231,7 +231,7 @@ func (_e *explorer) exploreInnerJoin(_rootState *exploreState, _root memo.ExprID
 									_e.mem.MemoizeDenormExpr(_root.Group, memo.Expr(_expr))
 									if _e.o.appliedRule != nil {
 										_after := _e.mem.ExprCount(_root.Group)
-										_e.o.appliedRule(opt.AssociateJoin, _root.Group, _after-_before)
+										_e.o.appliedRule(opt.AssociateJoin, _root.Group, _root.Expr, _after-_before)
 									}
 								}
 							}
@@ -298,7 +298,7 @@ func (_e *explorer) exploreSelect(_rootState *exploreState, _root memo.ExprID) (
 								_e.mem.MemoizeDenormExpr(_root.Group, memo.Expr(_expr))
 								if _e.o.appliedRule != nil {
 									_after := _e.mem.ExprCount(_root.Group)
-									_e.o.appliedRule(opt.PushDownGroupBy, _root.Group, _after-_before)
+									_e.o.appliedRule(opt.PushDownGroupBy, _root.Group, _root.Expr, _after-_before)
 								}
 							}
 						}
@@ -328,7 +328,7 @@ func (_e *explorer) exploreScan(_rootState *exploreState, _root memo.ExprID) (_f
 					}
 					if _e.o.appliedRule != nil {
 						_after := _e.mem.ExprCount(_root.Group)
-						_e.o.appliedRule(opt.GenerateIndexScans, _root.Group, _after-_before)
+						_e.o.appliedRule(opt.GenerateIndexScans, _root.Group, _root.Expr, _after-_before)
 					}
 				}
 			}
@@ -405,7 +405,7 @@ func (_e *explorer) exploreList(_rootState *exploreState, _root memo.ExprID) (_f
 																}
 																if _e.o.appliedRule != nil {
 																	_after := _e.mem.ExprCount(_root.Group)
-																	_e.o.appliedRule(opt.List, _root.Group, _after-_before)
+																	_e.o.appliedRule(opt.List, _root.Group, _root.Expr, _after-_before)
 																}
 															}
 														}
@@ -467,7 +467,7 @@ func (_e *explorer) exploreOp(_rootState *exploreState, _root memo.ExprID) (_ful
 									_e.mem.MemoizeDenormExpr(_root.Group, memo.Expr(_expr))
 									if _e.o.appliedRule != nil {
 										_after := _e.mem.ExprCount(_root.Group)
-										_e.o.appliedRule(opt.ListNot, _root.Group, _after-_before)
+										_e.o.appliedRule(opt.ListNot, _root.Group, _root.Expr, _after-_before)
 									}
 								}
 							}

diff --git a/pkg/sql/opt/optgen/cmd/optgen/testdata/factory b/pkg/sql/opt/optgen/cmd/optgen/testdata/factory
@@ -146,7 +146,7 @@ func (_f *Factory) ConstructInnerJoin(
 			)
 			_f.mem.AddAltFingerprint(_innerJoinExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.CommuteJoin, _group, 0)
+				_f.appliedRule(opt.CommuteJoin, _group, 0, 0)
 			}
 			return _group
 		}
@@ -273,7 +273,7 @@ func (_f *Factory) ConstructEq(
 								)
 								_f.mem.AddAltFingerprint(_eqExpr.Fingerprint(), _group)
 								if _f.appliedRule != nil {
-									_f.appliedRule(opt.NormalizeVarPlus, _group, 0)
+									_f.appliedRule(opt.NormalizeVarPlus, _group, 0, 0)
 								}
 								return _group
 							}
@@ -318,7 +318,7 @@ func (_f *Factory) ConstructLt(
 								)
 								_f.mem.AddAltFingerprint(_ltExpr.Fingerprint(), _group)
 								if _f.appliedRule != nil {
-									_f.appliedRule(opt.NormalizeVarPlus, _group, 0)
+									_f.appliedRule(opt.NormalizeVarPlus, _group, 0, 0)
 								}
 								return _group
 							}
@@ -491,7 +491,7 @@ func (_f *Factory) ConstructFunc(
 							)
 							_f.mem.AddAltFingerprint(_funcExpr.Fingerprint(), _group)
 							if _f.appliedRule != nil {
-								_f.appliedRule(opt.Concat, _group, 0)
+								_f.appliedRule(opt.Concat, _group, 0, 0)
 							}
 							return _group
 						}
@@ -642,7 +642,7 @@ func (_f *Factory) ConstructSelect(
 						)
 						_f.mem.AddAltFingerprint(_selectExpr.Fingerprint(), _group)
 						if _f.appliedRule != nil {
-							_f.appliedRule(opt.Test, _group, 0)
+							_f.appliedRule(opt.Test, _group, 0, 0)
 						}
 						return _group
 					}
@@ -829,7 +829,7 @@ func (_f *Factory) ConstructList(
 												_group = _f.funcs.Construct(any, first, last, single, empty)
 												_f.mem.AddAltFingerprint(_listExpr.Fingerprint(), _group)
 												if _f.appliedRule != nil {
-													_f.appliedRule(opt.List, _group, 0)
+													_f.appliedRule(opt.List, _group, 0, 0)
 												}
 												return _group
 											}
@@ -917,7 +917,7 @@ func (_f *Factory) ConstructJoin(
 			_group = _f.funcs.Construct(_f.mem.InternList([]memo.GroupID{}), _f.mem.InternList([]memo.GroupID{left}), _f.mem.InternList([]memo.GroupID{left, right}), _f.mem.InternList([]memo.GroupID{_f.mem.InternList([]memo.GroupID{on})}))
 			_f.mem.AddAltFingerprint(_joinExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.ConstructList, _group, 0)
+				_f.appliedRule(opt.ConstructList, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1022,7 +1022,7 @@ func (_f *Factory) ConstructOp(
 							)
 							_f.mem.AddAltFingerprint(_opExpr.Fingerprint(), _group)
 							if _f.appliedRule != nil {
-								_f.appliedRule(opt.ListNot, _group, 0)
+								_f.appliedRule(opt.ListNot, _group, 0, 0)
 							}
 							return _group
 						}
@@ -1144,7 +1144,7 @@ func (_f *Factory) ConstructEq(
 							_group = m.mem.InternPrivate(tree.NewDString("foo"))
 							_f.mem.AddAltFingerprint(_eqExpr.Fingerprint(), _group)
 							if _f.appliedRule != nil {
-								_f.appliedRule(opt.Constant, _group, 0)
+								_f.appliedRule(opt.Constant, _group, 0, 0)
 							}
 							return _group
 						}
@@ -1193,7 +1193,7 @@ func (_f *Factory) ConstructNe(
 							_group = m.mem.InternPrivate(tree.NewDString("foo"))
 							_f.mem.AddAltFingerprint(_neExpr.Fingerprint(), _group)
 							if _f.appliedRule != nil {
-								_f.appliedRule(opt.Dynamic, _group, 0)
+								_f.appliedRule(opt.Dynamic, _group, 0, 0)
 							}
 							return _group
 						}
@@ -1294,7 +1294,7 @@ func (_f *Factory) ConstructPlus(
 					_group = _f.ConstructNull()
 					_f.mem.AddAltFingerprint(_plusExpr.Fingerprint(), _group)
 					if _f.appliedRule != nil {
-						_f.appliedRule(opt.Fold, _group, 0)
+						_f.appliedRule(opt.Fold, _group, 0, 0)
 					}
 					return _group
 				}
@@ -1325,7 +1325,7 @@ func (_f *Factory) ConstructMinus(
 					_group = _f.ConstructNull()
 					_f.mem.AddAltFingerprint(_minusExpr.Fingerprint(), _group)
 					if _f.appliedRule != nil {
-						_f.appliedRule(opt.Fold, _group, 0)
+						_f.appliedRule(opt.Fold, _group, 0, 0)
 					}
 					return _group
 				}
@@ -1490,7 +1490,7 @@ func (_f *Factory) ConstructNot(
 						_group = _f.funcs.Invert(_f.mem.NormExpr(input).Operator(), left, right)
 						_f.mem.AddAltFingerprint(_notExpr.Fingerprint(), _group)
 						if _f.appliedRule != nil {
-							_f.appliedRule(opt.Invert, _group, 0)
+							_f.appliedRule(opt.Invert, _group, 0, 0)
 						}
 						return _group
 					}
@@ -1597,7 +1597,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject1, _group, 0)
+				_f.appliedRule(opt.EliminateProject1, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1609,7 +1609,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject2, _group, 0)
+				_f.appliedRule(opt.EliminateProject2, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1621,7 +1621,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject3, _group, 0)
+				_f.appliedRule(opt.EliminateProject3, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1633,7 +1633,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject4, _group, 0)
+				_f.appliedRule(opt.EliminateProject4, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1645,7 +1645,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject5, _group, 0)
+				_f.appliedRule(opt.EliminateProject5, _group, 0, 0)
 			}
 			return _group
 		}
@@ -1657,7 +1657,7 @@ func (_f *Factory) ConstructProject(
 			_group = input
 			_f.mem.AddAltFingerprint(_projectExpr.Fingerprint(), _group)
 			if _f.appliedRule != nil {
-				_f.appliedRule(opt.EliminateProject6, _group, 0)
+				_f.appliedRule(opt.EliminateProject6, _group, 0, 0)
 			}
 			return _group
 		}

diff --git a/pkg/sql/opt/optgen/cmd/optgen/testdata/rulenames b/pkg/sql/opt/optgen/cmd/optgen/testdata/rulenames
@@ -34,6 +34,10 @@ const (
 	EliminateEmptyAnd
 	EliminateSingletonAnd
 
+	// startExploreRule tracks the number of normalization rules;
+	// all rules greater than this value are exploration rules.
+	startExploreRule
+
 	// ------------------------------------------------------------
 	// Explore Rule Names
 	// ------------------------------------------------------------

diff --git a/pkg/sql/opt/rule_name.go b/pkg/sql/opt/rule_name.go
@@ -37,6 +37,18 @@ const (
 	NumManualRuleNames
 )
 
+// IsNormalize returns true if r is a normalization rule.
+func (r RuleName) IsNormalize() bool {
+	return r < startExploreRule
+}
+
+// IsExplore returns true if r is an exploration rule.
+func (r RuleName) IsExplore() bool {
+	return r > startExploreRule
+}
+
 // Make linter happy.
 var _ = InvalidRuleName
 var _ = NumManualRuleNames
+var _ = RuleName.IsNormalize
+var _ = RuleName.IsExplore
diff --git a/pkg/sql/opt/rule_name_string.go b/pkg/sql/opt/rule_name_string.go