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
radondb · Dec 2, 2019 · 8f7a4c5 · 8f7a4c5
1 parent b689ebe
commit 8f7a4c5
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
 	}