Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

planner: don't recompute the hashcode when generated column substitution doesn't happen #46450

Merged
merged 3 commits into from
Sep 4, 2023
Merged

planner: don't recompute the hashcode when generated column substitution doesn't happen #46450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cafe6d8
don't recompute the hashcode when generated column substitution doesn…
AilinKid Aug 28, 2023
6b59271
make fmt
AilinKid Aug 28, 2023
a57b178
make bazel
AilinKid Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions planner/core/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ go_test(
"planbuilder_test.go",
"point_get_plan_test.go",
"preprocess_test.go",
"rule_generate_column_substitute_test.go",
"rule_join_reorder_dp_test.go",
"rule_join_reorder_test.go",
"rule_result_reorder_test.go",
Expand Down
20 changes: 20 additions & 0 deletions planner/core/logical_plans_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,26 @@ type plannerSuite struct {
ctx sessionctx.Context
}

func (p *plannerSuite) GetParser() *parser.Parser {
return p.p
}

func (p *plannerSuite) GetIS() infoschema.InfoSchema {
return p.is
}

func (p *plannerSuite) GetCtx() sessionctx.Context {
return p.ctx
}

func CreatePlannerSuite(sctx sessionctx.Context, is infoschema.InfoSchema) (s *plannerSuite) {
s = new(plannerSuite)
s.is = is
s.p = parser.New()
s.ctx = sctx
return s
}

func createPlannerSuite() (s *plannerSuite) {
s = new(plannerSuite)
tblInfos := []*model.TableInfo{
Expand Down
40 changes: 28 additions & 12 deletions planner/core/rule_generate_column_substitute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,15 @@
}
}

func tryToSubstituteExpr(expr *expression.Expression, lp LogicalPlan, candidateExpr expression.Expression, tp types.EvalType, schema *expression.Schema, col *expression.Column, opt *logicalOptimizeOp) {
func tryToSubstituteExpr(expr *expression.Expression, lp LogicalPlan, candidateExpr expression.Expression, tp types.EvalType, schema *expression.Schema, col *expression.Column, opt *logicalOptimizeOp) bool {
changed := false
if (*expr).Equal(lp.SCtx(), candidateExpr) && candidateExpr.GetType().EvalType() == tp &&
schema.ColumnIndex(col) != -1 {
*expr = col
appendSubstituteColumnStep(lp, candidateExpr, col, opt)
changed = true
}
return changed
}

func appendSubstituteColumnStep(lp LogicalPlan, candidateExpr expression.Expression, col *expression.Column, opt *logicalOptimizeOp) {
Expand All @@ -97,26 +100,38 @@
opt.appendStepToCurrent(lp.ID(), lp.TP(), reason, action)
}

func substituteExpression(cond expression.Expression, lp LogicalPlan, exprToColumn ExprColumnMap, schema *expression.Schema, opt *logicalOptimizeOp) {
// SubstituteExpression is Exported for bench
func SubstituteExpression(cond expression.Expression, lp LogicalPlan, exprToColumn ExprColumnMap, schema *expression.Schema, opt *logicalOptimizeOp) bool {
return substituteExpression(cond, lp, exprToColumn, schema, opt)

Check warning on line 105 in planner/core/rule_generate_column_substitute.go

View check run for this annotation

Codecov / codecov/patch

planner/core/rule_generate_column_substitute.go#L104-L105 

Added lines #L104 - L105 were not covered by tests
}

func substituteExpression(cond expression.Expression, lp LogicalPlan, exprToColumn ExprColumnMap, schema *expression.Schema, opt *logicalOptimizeOp) bool {
sf, ok := cond.(*expression.ScalarFunction)
if !ok {
return
return false

Check warning on line 111 in planner/core/rule_generate_column_substitute.go

View check run for this annotation

Codecov / codecov/patch

planner/core/rule_generate_column_substitute.go#L111 

Added line #L111 was not covered by tests
}
sctx := lp.SCtx().GetSessionVars().StmtCtx
changed := false
collectChanged := func(partial bool) {
if partial && !changed {
changed = true
}
}
defer func() {
// If the argument is not changed, hash code doesn't need to recount again.
// But we always do it to keep the code simple and stupid.
expression.ReHashCode(sf, sctx)
if changed {
expression.ReHashCode(sf, sctx)
Copy link
Contributor Author

@AilinKid AilinKid Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for better performance, we could even just clean the hashcode as [:0] here because the computed hashcode for the toper/topest OR expression is never being used. Let it be computed as lazily as it encounters the needs of the hashcode of flattenDNFItem

}
}()
var expr *expression.Expression
var tp types.EvalType
switch sf.FuncName.L {
case ast.EQ, ast.LT, ast.LE, ast.GT, ast.GE:
for candidateExpr, column := range exprToColumn {
tryToSubstituteExpr(&sf.GetArgs()[1], lp, candidateExpr, sf.GetArgs()[0].GetType().EvalType(), schema, column, opt)
collectChanged(tryToSubstituteExpr(&sf.GetArgs()[1], lp, candidateExpr, sf.GetArgs()[0].GetType().EvalType(), schema, column, opt))
}
for candidateExpr, column := range exprToColumn {
tryToSubstituteExpr(&sf.GetArgs()[0], lp, candidateExpr, sf.GetArgs()[1].GetType().EvalType(), schema, column, opt)
collectChanged(tryToSubstituteExpr(&sf.GetArgs()[0], lp, candidateExpr, sf.GetArgs()[1].GetType().EvalType(), schema, column, opt))
}
case ast.In:
expr = &sf.GetArgs()[0]
Expand All @@ -132,21 +147,22 @@
}
if canSubstitute {
for candidateExpr, column := range exprToColumn {
tryToSubstituteExpr(expr, lp, candidateExpr, tp, schema, column, opt)
collectChanged(tryToSubstituteExpr(expr, lp, candidateExpr, tp, schema, column, opt))
}
}
case ast.Like:
expr = &sf.GetArgs()[0]
tp = sf.GetArgs()[1].GetType().EvalType()
for candidateExpr, column := range exprToColumn {
tryToSubstituteExpr(expr, lp, candidateExpr, tp, schema, column, opt)
collectChanged(tryToSubstituteExpr(expr, lp, candidateExpr, tp, schema, column, opt))
}
case ast.LogicOr, ast.LogicAnd:
substituteExpression(sf.GetArgs()[0], lp, exprToColumn, schema, opt)
substituteExpression(sf.GetArgs()[1], lp, exprToColumn, schema, opt)
collectChanged(substituteExpression(sf.GetArgs()[0], lp, exprToColumn, schema, opt))
collectChanged(substituteExpression(sf.GetArgs()[1], lp, exprToColumn, schema, opt))
case ast.UnaryNot:
substituteExpression(sf.GetArgs()[0], lp, exprToColumn, schema, opt)
collectChanged(substituteExpression(sf.GetArgs()[0], lp, exprToColumn, schema, opt))
}
return changed
}

func (gc *gcSubstituter) substitute(ctx context.Context, lp LogicalPlan, exprToColumn ExprColumnMap, opt *logicalOptimizeOp) LogicalPlan {
Expand Down
Loading