From 20f6b049871f2bde4951c1190fae9e31620ba035 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Tue, 3 Jul 2018 11:43:02 -0400 Subject: [PATCH 1/3] opt: allow distinguishing between norm/explore rules Release note: None --- pkg/sql/opt/optgen/cmd/optgen/rule_names_gen.go | 3 +++ pkg/sql/opt/optgen/cmd/optgen/testdata/rulenames | 4 ++++ pkg/sql/opt/rule_name.go | 12 ++++++++++++ pkg/sql/opt/rule_name_string.go | 4 ++-- 4 files changed, 21 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 a3369328034c..2ea0d16b1b67 100644 --- 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/rulenames b/pkg/sql/opt/optgen/cmd/optgen/testdata/rulenames index 91ef4b8b42f9..70663fdbb9b3 100644 --- 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 index fed91b661db1..ca081bd955e3 100644 --- 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 index 18aa69335978..08297e326908 100644 --- a/pkg/sql/opt/rule_name_string.go +++ b/pkg/sql/opt/rule_name_string.go @@ -4,9 +4,9 @@ package opt import "strconv" -const _RuleName_name = "InvalidRuleNameSimplifyProjectOrderingSimplifyRootOrderingPruneRootColsNumManualRuleNamesEliminateEmptyAndEliminateEmptyOrEliminateSingletonAndOrSimplifyAndSimplifyOrSimplifyFiltersFoldNullAndOrNegateComparisonEliminateNotNegateAndNegateOrExtractRedundantClauseExtractRedundantSubclauseCommuteVarInequalityCommuteConstInequalityNormalizeCmpPlusConstNormalizeCmpMinusConstNormalizeCmpConstMinusNormalizeTupleEqualityFoldNullComparisonLeftFoldNullComparisonRightFoldIsNullFoldNonNullIsNullFoldIsNotNullFoldNonNullIsNotNullCommuteNullIsDecorrelateJoinTryDecorrelateSelectTryDecorrelateProjectTryDecorrelateProjectSelectTryDecorrelateScalarGroupByHoistSelectExistsHoistSelectNotExistsHoistSelectSubqueryHoistProjectSubqueryHoistJoinSubqueryHoistValuesSubqueryNormalizeAnyFilterNormalizeNotAnyFilterEliminateDistinctEliminateGroupByProjectPushSelectIntoInlinableProjectInlineProjectInProjectEnsureJoinFiltersAndEnsureJoinFiltersPushFilterIntoJoinLeftAndRightMapFilterIntoJoinLeftMapFilterIntoJoinRightPushFilterIntoJoinLeftPushFilterIntoJoinRightSimplifyLeftJoinSimplifyRightJoinEliminateSemiJoinEliminateAntiJoinEliminateJoinNoColsLeftEliminateJoinNoColsRightEliminateLimitPushLimitIntoProjectPushOffsetIntoProjectEliminateMax1RowFoldPlusZeroFoldZeroPlusFoldMinusZeroFoldMultOneFoldOneMultFoldDivOneInvertMinusEliminateUnaryMinusFoldUnaryMinusSimplifyLimitOrderingSimplifyOffsetOrderingSimplifyGroupByOrderingSimplifyRowNumberOrderingSimplifyExplainOrderingEliminateProjectEliminateProjectProjectPruneProjectColsPruneScanColsPruneSelectColsPruneLimitColsPruneOffsetColsPruneJoinLeftColsPruneJoinRightColsPruneAggColsPruneGroupByColsPruneValuesColsPruneRowNumberColsPruneExplainColsCommuteVarCommuteConstEliminateCoalesceSimplifyCoalesceEliminateCastFoldNullCastFoldNullUnaryFoldNullBinaryLeftFoldNullBinaryRightFoldNullInNonEmptyFoldNullInEmptyFoldNullNotInEmptyNormalizeInConstFoldInNullEliminateExistsProjectEliminateExistsGroupByEliminateSelectEnsureSelectFiltersAndEnsureSelectFiltersMergeSelectsPushSelectIntoProjectSimplifySelectLeftJoinSimplifySelectRightJoinMergeSelectInnerJoinPushSelectIntoJoinLeftPushSelectIntoJoinRightPushSelectIntoGroupByRemoveNotNullConditionReplaceMinWithLimitGenerateMergeJoinsPushLimitIntoScanPushLimitIntoIndexJoinGenerateIndexScansConstrainScanPushFilterIntoIndexJoinNoRemainderPushFilterIntoIndexJoinConstrainIndexJoinScanNumRuleNames" +const _RuleName_name = "InvalidRuleNameSimplifyProjectOrderingSimplifyRootOrderingPruneRootColsNumManualRuleNamesEliminateEmptyAndEliminateEmptyOrEliminateSingletonAndOrSimplifyAndSimplifyOrSimplifyFiltersFoldNullAndOrNegateComparisonEliminateNotNegateAndNegateOrExtractRedundantClauseExtractRedundantSubclauseCommuteVarInequalityCommuteConstInequalityNormalizeCmpPlusConstNormalizeCmpMinusConstNormalizeCmpConstMinusNormalizeTupleEqualityFoldNullComparisonLeftFoldNullComparisonRightFoldIsNullFoldNonNullIsNullFoldIsNotNullFoldNonNullIsNotNullCommuteNullIsDecorrelateJoinTryDecorrelateSelectTryDecorrelateProjectTryDecorrelateProjectSelectTryDecorrelateScalarGroupByHoistSelectExistsHoistSelectNotExistsHoistSelectSubqueryHoistProjectSubqueryHoistJoinSubqueryHoistValuesSubqueryNormalizeAnyFilterNormalizeNotAnyFilterEliminateDistinctEliminateGroupByProjectPushSelectIntoInlinableProjectInlineProjectInProjectEnsureJoinFiltersAndEnsureJoinFiltersPushFilterIntoJoinLeftAndRightMapFilterIntoJoinLeftMapFilterIntoJoinRightPushFilterIntoJoinLeftPushFilterIntoJoinRightSimplifyLeftJoinSimplifyRightJoinEliminateSemiJoinEliminateAntiJoinEliminateJoinNoColsLeftEliminateJoinNoColsRightEliminateLimitPushLimitIntoProjectPushOffsetIntoProjectEliminateMax1RowFoldPlusZeroFoldZeroPlusFoldMinusZeroFoldMultOneFoldOneMultFoldDivOneInvertMinusEliminateUnaryMinusFoldUnaryMinusSimplifyLimitOrderingSimplifyOffsetOrderingSimplifyGroupByOrderingSimplifyRowNumberOrderingSimplifyExplainOrderingEliminateProjectEliminateProjectProjectPruneProjectColsPruneScanColsPruneSelectColsPruneLimitColsPruneOffsetColsPruneJoinLeftColsPruneJoinRightColsPruneAggColsPruneGroupByColsPruneValuesColsPruneRowNumberColsPruneExplainColsCommuteVarCommuteConstEliminateCoalesceSimplifyCoalesceEliminateCastFoldNullCastFoldNullUnaryFoldNullBinaryLeftFoldNullBinaryRightFoldNullInNonEmptyFoldNullInEmptyFoldNullNotInEmptyNormalizeInConstFoldInNullEliminateExistsProjectEliminateExistsGroupByEliminateSelectEnsureSelectFiltersAndEnsureSelectFiltersMergeSelectsPushSelectIntoProjectSimplifySelectLeftJoinSimplifySelectRightJoinMergeSelectInnerJoinPushSelectIntoJoinLeftPushSelectIntoJoinRightPushSelectIntoGroupByRemoveNotNullConditionstartExploreRuleReplaceMinWithLimitGenerateMergeJoinsPushLimitIntoScanPushLimitIntoIndexJoinGenerateIndexScansConstrainScanPushFilterIntoIndexJoinNoRemainderPushFilterIntoIndexJoinConstrainIndexJoinScanNumRuleNames" -var _RuleName_index = [...]uint16{0, 15, 38, 58, 71, 89, 106, 122, 145, 156, 166, 181, 194, 210, 222, 231, 239, 261, 286, 306, 328, 349, 371, 393, 415, 437, 460, 470, 487, 500, 520, 533, 548, 568, 589, 616, 643, 660, 680, 699, 719, 736, 755, 773, 794, 811, 834, 864, 886, 906, 923, 953, 974, 996, 1018, 1041, 1057, 1074, 1091, 1108, 1131, 1155, 1169, 1189, 1210, 1226, 1238, 1250, 1263, 1274, 1285, 1295, 1306, 1325, 1339, 1360, 1382, 1405, 1430, 1453, 1469, 1492, 1508, 1521, 1536, 1550, 1565, 1582, 1600, 1612, 1628, 1643, 1661, 1677, 1687, 1699, 1716, 1732, 1745, 1757, 1770, 1788, 1807, 1825, 1840, 1858, 1874, 1884, 1906, 1928, 1943, 1965, 1984, 1996, 2017, 2039, 2062, 2082, 2104, 2127, 2148, 2170, 2189, 2207, 2224, 2246, 2264, 2277, 2311, 2334, 2356, 2368} +var _RuleName_index = [...]uint16{0, 15, 38, 58, 71, 89, 106, 122, 145, 156, 166, 181, 194, 210, 222, 231, 239, 261, 286, 306, 328, 349, 371, 393, 415, 437, 460, 470, 487, 500, 520, 533, 548, 568, 589, 616, 643, 660, 680, 699, 719, 736, 755, 773, 794, 811, 834, 864, 886, 906, 923, 953, 974, 996, 1018, 1041, 1057, 1074, 1091, 1108, 1131, 1155, 1169, 1189, 1210, 1226, 1238, 1250, 1263, 1274, 1285, 1295, 1306, 1325, 1339, 1360, 1382, 1405, 1430, 1453, 1469, 1492, 1508, 1521, 1536, 1550, 1565, 1582, 1600, 1612, 1628, 1643, 1661, 1677, 1687, 1699, 1716, 1732, 1745, 1757, 1770, 1788, 1807, 1825, 1840, 1858, 1874, 1884, 1906, 1928, 1943, 1965, 1984, 1996, 2017, 2039, 2062, 2082, 2104, 2127, 2148, 2170, 2186, 2205, 2223, 2240, 2262, 2280, 2293, 2327, 2350, 2372, 2384} func (i RuleName) String() string { if i >= RuleName(len(_RuleName_index)-1) { From 130c9df814717268aec7d8f558bd55dd19cbd39c Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Tue, 3 Jul 2018 12:09:04 -0400 Subject: [PATCH 2/3] 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 --- pkg/sql/opt/memo/expr.go | 3 + pkg/sql/opt/norm/factory.go | 14 ++-- pkg/sql/opt/optgen/cmd/optgen/rule_gen.go | 4 +- .../opt/optgen/cmd/optgen/testdata/explorer | 12 ++-- .../opt/optgen/cmd/optgen/testdata/factory | 38 +++++------ pkg/sql/opt/testutils/forcing_opt.go | 67 ++++++++++++------- pkg/sql/opt/testutils/opt_steps.go | 15 +++-- pkg/sql/opt/xform/optimizer.go | 4 +- pkg/sql/opt/xform/physical_props.go | 2 +- 9 files changed, 97 insertions(+), 62 deletions(-) diff --git a/pkg/sql/opt/memo/expr.go b/pkg/sql/opt/memo/expr.go index ca3034d47134..4f0729522ae9 100644 --- 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 index d418f59608df..b866175c2da7 100644 --- 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 index b10993cf745a..9c0076ee990e 100644 --- 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/testdata/explorer b/pkg/sql/opt/optgen/cmd/optgen/testdata/explorer index 3f4896a28b96..e9c9a145fe58 100644 --- 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 index 48fda33b1a28..2742d5a881a4 100644 --- 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/testutils/forcing_opt.go b/pkg/sql/opt/testutils/forcing_opt.go index 7b3d127c0039..5b8c913205cc 100644 --- a/pkg/sql/opt/testutils/forcing_opt.go +++ b/pkg/sql/opt/testutils/forcing_opt.go @@ -38,12 +38,24 @@ type forcingOptimizer struct { // by the optimizer. lastMatched opt.RuleName - // lastApplied records the id of the expression that marks the portion of the - // tree affected by the most recent rule application. All expressions in the - // same memo group that are < lastApplied.Expr will assigned an infinite cost - // by the forcingCoster. Therefore, only expressions >= lastApplied.Expr can - // be in the output expression tree. - lastApplied memo.ExprID + // lastApplied records the name of the rule that was most recently applied by + // the optimizer. This is not necessarily the same with lastMatched because + // normalization rules can run in-between the match and the application of an + // exploration rule. + lastApplied opt.RuleName + + // lastAppliedGroup is the group where the last rule was applied (for both + // normalization and exploration rules). + lastAppliedGroup memo.GroupID + + // The following fields are only valid if lastApplied is an exploration rule: + // - lastExploreSourceExpr is the ordinal of the expression on which the last + // explore rule ran. + // - lastExploreNewExprs is the set of expression ordinals that were added by + // the rule (can be empty). + // All expressions are in lastAppliedGroup. + lastExploreSourceExpr memo.ExprOrdinal + lastExploreNewExprs util.FastIntSet } // newForcingOptimizer creates a forcing optimizer that stops applying any rules @@ -53,7 +65,6 @@ func newForcingOptimizer(tester *OptTester, steps int) (*forcingOptimizer, error o: xform.NewOptimizer(&tester.evalCtx), remaining: steps, lastMatched: opt.InvalidRuleName, - lastApplied: memo.InvalidExprID, } fo.o.NotifyOnMatchedRule(func(ruleName opt.RuleName) bool { @@ -67,22 +78,23 @@ func newForcingOptimizer(tester *OptTester, steps int) (*forcingOptimizer, error // Hook the AppliedRule notification in order to track the portion of the // expression tree affected by each transformation rule. - fo.lastApplied = memo.InvalidExprID - fo.o.NotifyOnAppliedRule(func(ruleName opt.RuleName, group memo.GroupID, added int) { - if added > 0 { - // This was an exploration rule that added one or more expressions to - // an existing group. Record the id of the first of those expressions. - // Previous expressions will be suppressed. - ord := memo.ExprOrdinal(fo.o.Memo().ExprCount(group) - added) - fo.lastApplied = memo.ExprID{Group: group, Expr: ord} - } else { - // This was a normalization that created a new memo group, or it was - // an exploration rule that didn't add any expressions to the group. - // Either way, none of the expressions in the group need to be - // suppressed. - fo.lastApplied = memo.MakeNormExprID(group) - } - }) + fo.o.NotifyOnAppliedRule( + func(ruleName opt.RuleName, group memo.GroupID, expr memo.ExprOrdinal, added int) { + fo.lastApplied = ruleName + fo.lastAppliedGroup = group + + if ruleName.IsExplore() { + // This was an exploration rule. Record the expression on which the rule + // was applied and the set of expressions that were generated. + fo.lastExploreSourceExpr = expr + fo.lastExploreNewExprs = util.FastIntSet{} + if added > 0 { + exprCount := fo.o.Memo().ExprCount(group) + fo.lastExploreNewExprs.AddRange(exprCount-added, exprCount-1) + } + } + }, + ) var err error fo.root, fo.required, err = tester.buildExpr(fo.o.Factory()) @@ -119,6 +131,15 @@ func (fo *forcingOptimizer) restrictToExprs( fo.o.SetCoster(coster) } +// restrictToGroup is a convenience variant of restrictToExprs when all +// expressions in a group are to be retained. +func (fo *forcingOptimizer) restrictToGroup(mem *memo.Memo, group memo.GroupID) { + coster := newForcingCoster(fo.o.Coster()) + + restrictToGroup(coster, mem, fo.root, group) + fo.o.SetCoster(coster) +} + // restrictToGroup walks the memo and adds expressions which need to be // suppressed to the forcingCoster so that the optimization result must contain // an expression in the given target group. diff --git a/pkg/sql/opt/testutils/opt_steps.go b/pkg/sql/opt/testutils/opt_steps.go index 183f8b6b688a..e784be4f7ea1 100644 --- a/pkg/sql/opt/testutils/opt_steps.go +++ b/pkg/sql/opt/testutils/opt_steps.go @@ -17,7 +17,6 @@ package testutils import ( "github.com/cockroachdb/cockroach/pkg/sql/opt" "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" - "github.com/cockroachdb/cockroach/pkg/util" ) // optSteps implements the stepping algorithm used by the OptTester's OptSteps @@ -136,9 +135,17 @@ func (os *optSteps) next() error { return err } - var exprs util.FastIntSet - exprs.AddRange(int(fo.lastApplied.Expr), fo.o.Memo().ExprCount(fo.lastApplied.Group)-1) - fo2.restrictToExprs(fo.o.Memo(), fo.lastApplied.Group, exprs) + if fo.lastApplied.IsNormalize() || fo.lastExploreNewExprs.Empty() { + // This was a normalization that created a new memo group, or it was + // an exploration rule that didn't add any expressions to the group. + // Either way, none of the expressions in the group need to be + // suppressed. + fo2.restrictToGroup(fo.o.Memo(), fo.lastAppliedGroup) + } else { + // This was an exploration rule that added one or more expressions to + // an existing group. Suppress all other expressions in the group. + fo2.restrictToExprs(fo.o.Memo(), fo.lastAppliedGroup, fo.lastExploreNewExprs) + } os.ev = fo2.optimize() } diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index c548190f94a6..488b1f066cb6 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -531,7 +531,7 @@ func (o *Optimizer) optimizeRootWithProps( rootProps = &simplified if o.appliedRule != nil { - o.appliedRule(opt.SimplifyRootOrdering, root, 0) + o.appliedRule(opt.SimplifyRootOrdering, root, 0, 0) } } } @@ -547,7 +547,7 @@ func (o *Optimizer) optimizeRootWithProps( if o.matchedRule == nil || o.matchedRule(opt.PruneRootCols) { root = o.f.CustomFuncs().PruneCols(root, neededCols) if o.appliedRule != nil { - o.appliedRule(opt.PruneRootCols, root, 0) + o.appliedRule(opt.PruneRootCols, root, 0, 0) } } } diff --git a/pkg/sql/opt/xform/physical_props.go b/pkg/sql/opt/xform/physical_props.go index 95026106ad66..a4fdc8255294 100644 --- a/pkg/sql/opt/xform/physical_props.go +++ b/pkg/sql/opt/xform/physical_props.go @@ -228,7 +228,7 @@ func (o *Optimizer) optimizeProjectOrdering(project *memo.ProjectExpr, physical physical.Ordering.Simplify(&relational.FuncDeps) if o.appliedRule != nil { - o.appliedRule(opt.SimplifyProjectOrdering, project.Input(), 0) + o.appliedRule(opt.SimplifyProjectOrdering, project.Input(), 0, 0) } } } From 445b2fcb9ea45dfde060533637ba85c8573039da Mon Sep 17 00:00:00 2001 From: Andrew Couch Date: Tue, 3 Jul 2018 13:52:35 -0400 Subject: [PATCH 3/3] ui: make sure we identify the cluster 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 --- pkg/ui/src/redux/analytics.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/ui/src/redux/analytics.ts b/pkg/ui/src/redux/analytics.ts index f20bdf4bf9d5..2859b81b5fd1 100644 --- a/pkg/ui/src/redux/analytics.ts +++ b/pkg/ui/src/redux/analytics.ts @@ -255,6 +255,8 @@ history.listen((location) => { } lastPageLocation = location; analytics.page(location); + // Identify the cluster. + analytics.identify(); }); // Record the initial page that was accessed; listen won't fire for the first