From 8f7a4c5796daf6900ac3fdcaaa17c6a7799e7a0a Mon Sep 17 00:00:00 2001 From: andy Date: Thu, 28 Nov 2019 20:50:41 +0800 Subject: [PATCH] planner/builder: make the code easy to understand #480 [summary] replace the verbose variables with meaningful variables [test case] N/A [patch codecov] N/A --- src/planner/builder/builder.go | 2 +- src/planner/builder/expr.go | 14 +++++++------- src/planner/builder/expr_test.go | 4 ++-- src/planner/builder/plan_node.go | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/planner/builder/builder.go b/src/planner/builder/builder.go index aee3c3af..3832f4a2 100644 --- a/src/planner/builder/builder.go +++ b/src/planner/builder/builder.go @@ -86,7 +86,7 @@ func processSelect(log *xlog.Log, router *router.Router, database string, node * } if node.Having != nil { - if err = pushHavings(root, node.Having.Expr, tbInfos); err != nil { + if err = pushHavings(root, node.Having.Expr); err != nil { return nil, err } } diff --git a/src/planner/builder/expr.go b/src/planner/builder/expr.go index 0af396d9..c73e54cb 100644 --- a/src/planner/builder/expr.go +++ b/src/planner/builder/expr.go @@ -317,7 +317,7 @@ func checkJoinOn(lpn, rpn PlanNode, join exprInfo) (exprInfo, error) { // parseHaving used to check the having exprs and parse into tuples. // unsupport: `select sum(t2.id) as tmp, t1.id from t2,t1 having tmp=1`. -func parseHaving(exprs sqlparser.Expr, tbInfos map[string]*tableInfo, fields []selectTuple) ([]exprInfo, error) { +func parseHaving(exprs sqlparser.Expr, fields []selectTuple) ([]exprInfo, error) { var tuples []exprInfo filters := splitAndExpression(nil, exprs) for _, filter := range filters { @@ -394,18 +394,18 @@ func getTbsInExpr(expr sqlparser.Expr) []string { func replaceCol(info exprInfo, colMap map[string]selectTuple) (exprInfo, error) { var tables []string var columns []*sqlparser.ColName - for _, col := range info.cols { - field, err := getMatchedField(col.Name.String(), colMap) + for _, old := range info.cols { + new, err := getMatchedField(old.Name.String(), colMap) if err != nil { return info, err } - if field.aggrFuc != "" { + if new.aggrFuc != "" { return info, errors.New("unsupported: aggregation.field.in.subquery.is.used.in.clause") } - info.expr = sqlparser.ReplaceExpr(info.expr, col, field.info.expr) - columns = append(columns, field.info.cols...) - for _, referTable := range field.info.referTables { + info.expr = sqlparser.ReplaceExpr(info.expr, old, new.info.expr) + columns = append(columns, new.info.cols...) + for _, referTable := range new.info.referTables { if !isContainKey(tables, referTable) { tables = append(tables, referTable) } diff --git a/src/planner/builder/expr_test.go b/src/planner/builder/expr_test.go index 2409b7a6..c61633b3 100644 --- a/src/planner/builder/expr_test.go +++ b/src/planner/builder/expr_test.go @@ -222,7 +222,7 @@ func TestParserHaving(t *testing.T) { err = p.pushSelectExprs(fields, nil, sel, aggTyp) assert.Nil(t, err) - err = pushHavings(p, sel.Having.Expr, p.getReferTables()) + err = pushHavings(p, sel.Having.Expr) assert.Nil(t, err) } } @@ -261,7 +261,7 @@ func TestParserHavingError(t *testing.T) { err = p.pushSelectExprs(fields, nil, sel, aggTyp) assert.Nil(t, err) - err = pushHavings(p, sel.Having.Expr, p.getReferTables()) + err = pushHavings(p, sel.Having.Expr) got := err.Error() assert.Equal(t, wants[i], got) } diff --git a/src/planner/builder/plan_node.go b/src/planner/builder/plan_node.go index ee2030fc..87799b93 100644 --- a/src/planner/builder/plan_node.go +++ b/src/planner/builder/plan_node.go @@ -116,8 +116,8 @@ func pushFilters(s PlanNode, expr sqlparser.Expr) (PlanNode, error) { } // pushHavings push a HAVING clause down. -func pushHavings(s PlanNode, expr sqlparser.Expr, tbInfos map[string]*tableInfo) error { - havings, err := parseHaving(expr, tbInfos, s.getFields()) +func pushHavings(s PlanNode, expr sqlparser.Expr) error { + havings, err := parseHaving(expr, s.getFields()) if err != nil { return err }