Skip to content

Commit

Permalink
planner: rewrite handleOthers in JoinNode radondb#517
Browse files Browse the repository at this point in the history 
[summary]
Make full use of the info parameter in selectTuple. Remove the idx in JoinNode instead of aliasIndex in mergenode, because the index may repeat.
[test case]
src/planner/builder/builder_test.go
src/planner/builder/from_test.go
src/planner/select_plan_test.go
[patch codecov]
src/planner/builder 97.9%
  • Loading branch information
@zhyass @BohuTANG
zhyass authored and BohuTANG committed Nov 7, 2019
1 parent 6847504 commit 0cf4b9e
Show file tree
Hide file tree
Showing 13 changed files with 176 additions and 204 deletions.
10 changes: 5 additions & 5 deletions src/planner/builder/aggregate_plan_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestAggregatePlan(t *testing.T) {
assert.Nil(t, err)
p, err := scanTableExprs(log, route, "sbtest", node.From)
assert.Nil(t, err)
tuples, aggTyp, err := parserSelectExprs(node.SelectExprs, p)
tuples, aggTyp, err := parseSelectExprs(node.SelectExprs, p)
assert.Nil(t, err)
assert.Equal(t, canPush, aggTyp)
_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestAggregatePlanUpperCase(t *testing.T) {
assert.Nil(t, err)
p, err := scanTableExprs(log, route, "sbtest", node.From)
assert.Nil(t, err)
tuples, aggTyp, err := parserSelectExprs(node.SelectExprs, p)
tuples, aggTyp, err := parseSelectExprs(node.SelectExprs, p)
assert.Nil(t, err)
assert.Equal(t, canPush, aggTyp)
_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestAggregatePlanHaving(t *testing.T) {
assert.Nil(t, err)
p, err := scanTableExprs(log, route, "sbtest", node.From)
assert.Nil(t, err)
tuples, aggTyp, err := parserSelectExprs(node.SelectExprs, p)
tuples, aggTyp, err := parseSelectExprs(node.SelectExprs, p)
assert.Nil(t, err)
assert.Equal(t, canPush, aggTyp)
_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -385,7 +385,7 @@ func TestAggregatePlanUnsupported(t *testing.T) {
node := tree.(*sqlparser.Select)
p, err := scanTableExprs(log, route, "sbtest", node.From)
assert.Nil(t, err)
tuples, aggTyp, err := parserSelectExprs(node.SelectExprs, p)
tuples, aggTyp, err := parseSelectExprs(node.SelectExprs, p)
assert.Nil(t, err)
assert.Equal(t, canPush, aggTyp)
_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -480,7 +480,7 @@ func TestAggregatePlans(t *testing.T) {
assert.Nil(t, err)
p, err := scanTableExprs(log, route, "sbtest", node.From)
assert.Nil(t, err)
tuples, aggTyp, err := parserSelectExprs(node.SelectExprs, p)
tuples, aggTyp, err := parseSelectExprs(node.SelectExprs, p)
assert.Nil(t, err)
//assert.Equal(t, canPush, aggTyp)
_, ok := p.(*MergeNode)
Expand Down
6 changes: 3 additions & 3 deletions src/planner/builder/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func processSelect(log *xlog.Log, router *router.Router, database string, node *

tbInfos := root.getReferTables()
if node.Where != nil {
joins, filters, err := parserWhereOrJoinExprs(node.Where.Expr, tbInfos)
joins, filters, err := parseWhereOrJoinExprs(node.Where.Expr, tbInfos)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -75,7 +75,7 @@ func processSelect(log *xlog.Log, router *router.Router, database string, node *
root.pushMisc(node)

var groups []selectTuple
fields, aggTyp, err := parserSelectExprs(node.SelectExprs, root)
fields, aggTyp, err := parseSelectExprs(node.SelectExprs, root)
if err != nil {
return nil, err
}
Expand All @@ -93,7 +93,7 @@ func processSelect(log *xlog.Log, router *router.Router, database string, node *
}

if node.Having != nil {
havings, err := parserHaving(node.Having.Expr, tbInfos)
havings, err := parseHaving(node.Having.Expr, tbInfos)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion src/planner/builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestProcessSelect(t *testing.T) {
project: "id",
out: []xcontext.QueryTuple{
{
Query: "select A.id, A.a + 1 as tmpo_0 from sbtest.A6 as A where A.id = 1 order by tmpo_0 asc",
Query: "select A.id, A.a + 1 as tmpc_0 from sbtest.A6 as A where A.id = 1 order by tmpc_0 asc",
Backend: "backend6",
Range: "[512-4096)",
},
Expand Down Expand Up @@ -597,6 +597,7 @@ func TestSelectSupported(t *testing.T) {
"select /*+nested+*/ A.id from G,A,B where A.id=B.id having G.id=B.id and B.a=1 and 1=1",
"select COALESCE(A.b, ''), IF(A.b IS NULL, FALSE, TRUE) AS spent from A left join B on A.a=B.a",
"select COALESCE(B.b, ''), IF(B.b IS NULL, FALSE, TRUE) AS spent from A join B on A.a=B.a",
"select A.id from A left join B on B.id+1=A.id where B.str1+B.str2 is null",
}

log := xlog.NewStdLog(xlog.Level(xlog.PANIC))
Expand Down
25 changes: 4 additions & 21 deletions src/planner/builder/expr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ type exprInfo struct {
vals []*sqlparser.SQLVal
}

// parserWhereOrJoinExprs parser exprs in where or join on conditions.
// parseWhereOrJoinExprs parse exprs in where or join on conditions.
// eg: 't1.a=t2.a and t1.b=2'.
// t1.a=t2.a paser in joins.
// t1.b=2 paser in wheres, t1.b col, 2 val.
func parserWhereOrJoinExprs(exprs sqlparser.Expr, tbInfos map[string]*tableInfo) ([]exprInfo, []exprInfo, error) {
func parseWhereOrJoinExprs(exprs sqlparser.Expr, tbInfos map[string]*tableInfo) ([]exprInfo, []exprInfo, error) {
filters := splitAndExpression(nil, exprs)
var joins, wheres []exprInfo

Expand Down Expand Up @@ -263,23 +263,6 @@ func convertOrToIn(node sqlparser.Expr) sqlparser.Expr {
return result
}

// getTbInExpr used to get the tbs from the expr.
func getTbInExpr(expr sqlparser.Expr) []string {
var referTables []string
sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
switch node := node.(type) {
case *sqlparser.ColName:
tableName := node.Qualifier.Name.String()
if isContainKey(tableName, referTables) {
return true, nil
}
referTables = append(referTables, tableName)
}
return true, nil
}, expr)
return referTables
}

// convertToLeftJoin converts a right join into a left join.
func convertToLeftJoin(joinExpr *sqlparser.JoinTableExpr) {
newExpr := joinExpr.LeftExpr
Expand Down Expand Up @@ -311,9 +294,9 @@ func checkJoinOn(lpn, rpn PlanNode, join exprInfo) (exprInfo, error) {
return join, nil
}

// parserHaving used to check the having exprs and parser into tuples.
// parseHaving used to check the having exprs and parse into tuples.
// unsupport: `select t2.id as tmp, t1.id from t2,t1 having tmp=1`.
func parserHaving(exprs sqlparser.Expr, tbInfos map[string]*tableInfo) ([]exprInfo, error) {
func parseHaving(exprs sqlparser.Expr, tbInfos map[string]*tableInfo) ([]exprInfo, error) {
filters := splitAndExpression(nil, exprs)
var tuples []exprInfo

Expand Down
28 changes: 14 additions & 14 deletions src/planner/builder/expr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestParserSelectExprsSubquery(t *testing.T) {
sel := node.(*sqlparser.Select)
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)
_, _, err = parserSelectExprs(sel.SelectExprs, p)
_, _, err = parseSelectExprs(sel.SelectExprs, p)
got := err.Error()
assert.Equal(t, want, got)
}
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestParserWhereOrJoinExprs(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

_, _, err = parserWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
_, _, err = parseWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
assert.Nil(t, err)
}
}
Expand Down Expand Up @@ -128,7 +128,7 @@ func TestWhereFilters(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

joins, filters, err := parserWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
joins, filters, err := parseWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
assert.Nil(t, err)

err = p.pushFilter(filters)
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestWhereFiltersError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

joins, filters, err := parserWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
joins, filters, err := parseWhereOrJoinExprs(sel.Where.Expr, p.getReferTables())
assert.Nil(t, err)

err = p.pushFilter(filters)
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestCheckGroupBy(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -263,7 +263,7 @@ func TestCheckGroupByError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -302,7 +302,7 @@ func TestCheckDistinct(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -339,7 +339,7 @@ func TestCheckDistinctError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -374,7 +374,7 @@ func TestSelectExprs(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, aggTyp, err := parserSelectExprs(sel.SelectExprs, p)
fields, aggTyp, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -417,7 +417,7 @@ func TestSelectExprsError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, aggTyp, err := parserSelectExprs(sel.SelectExprs, p)
fields, aggTyp, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)

_, ok := p.(*MergeNode)
Expand Down Expand Up @@ -454,7 +454,7 @@ func TestParserHaving(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

havings, err := parserHaving(sel.Having.Expr, p.getReferTables())
havings, err := parseHaving(sel.Having.Expr, p.getReferTables())
assert.Nil(t, err)

err = p.pushHaving(havings)
Expand Down Expand Up @@ -492,7 +492,7 @@ func TestParserHavingError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

havings, err := parserHaving(sel.Having.Expr, p.getReferTables())
havings, err := parseHaving(sel.Having.Expr, p.getReferTables())
if err != nil {
got := err.Error()
assert.Equal(t, wants[i], got)
Expand Down Expand Up @@ -528,7 +528,7 @@ func TestPushOrderBy(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)
switch p := p.(type) {
case *MergeNode:
Expand Down Expand Up @@ -570,7 +570,7 @@ func TestPushOrderByError(t *testing.T) {
p, err := scanTableExprs(log, route, database, sel.From)
assert.Nil(t, err)

fields, _, err := parserSelectExprs(sel.SelectExprs, p)
fields, _, err := parseSelectExprs(sel.SelectExprs, p)
assert.Nil(t, err)
switch p := p.(type) {
case *MergeNode:
Expand Down
2 changes: 1 addition & 1 deletion src/planner/builder/from.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func join(log *xlog.Log, lpn, rpn SelectNode, joinExpr *sqlparser.JoinTableExpr,
if joinExpr.On == nil {
joinExpr = nil
} else {
if joinOn, otherJoinOn, err = parserWhereOrJoinExprs(joinExpr.On, referTables); err != nil {
if joinOn, otherJoinOn, err = parseWhereOrJoinExprs(joinExpr.On, referTables); err != nil {
return nil, err
}
for i, jt := range joinOn {
Expand Down
18 changes: 6 additions & 12 deletions src/planner/builder/from_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ func TestScanTableExprs(t *testing.T) {
assert.Equal(t, 0, len(m.index))
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
got := err.Error()
assert.Equal(t, "unsupported: clause.'A.b + B.b > 0'.in.cross-shard.join", got)
}
Expand All @@ -143,8 +142,7 @@ func TestScanTableExprs(t *testing.T) {
assert.Equal(t, 0, len(j.tableFilter))
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
assert.Nil(t, err)
}
// right join2.
Expand Down Expand Up @@ -177,8 +175,7 @@ func TestScanTableExprs(t *testing.T) {
assert.True(t, m.hasParen)
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
assert.Nil(t, err)
}
// can merge shard tables.
Expand Down Expand Up @@ -439,8 +436,7 @@ func TestScanTableExprsList(t *testing.T) {
assert.Equal(t, 0, len(m.index))
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
got := err.Error()
assert.Equal(t, "unsupported: clause.'A.b + L.b > 0'.in.cross-shard.join", got)
}
Expand All @@ -462,8 +458,7 @@ func TestScanTableExprsList(t *testing.T) {
assert.Equal(t, 0, len(j.tableFilter))
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
assert.Nil(t, err)
}
// right join2.
Expand Down Expand Up @@ -496,8 +491,7 @@ func TestScanTableExprsList(t *testing.T) {
assert.True(t, m.hasParen)
assert.NotNil(t, j.otherJoinOn)

i := 0
err = j.pushOtherJoin(&i)
err = j.pushOtherJoin()
assert.Nil(t, err)
}
// can merge shard tables.
Expand Down
Loading

0 comments on commit 0cf4b9e

Please sign in to comment.