Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Check projection aliases when assigned to index. #640

Merged
merged 2 commits into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
58 changes: 53 additions & 5 deletions engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"testing"
"time"

"gopkg.in/src-d/go-mysql-server.v0"
sqle "gopkg.in/src-d/go-mysql-server.v0"
"gopkg.in/src-d/go-mysql-server.v0/auth"
"gopkg.in/src-d/go-mysql-server.v0/mem"
"gopkg.in/src-d/go-mysql-server.v0/sql"
Expand Down Expand Up @@ -1638,22 +1638,32 @@ func TestIndexes(t *testing.T) {

_, _, err = e.Query(
newCtx(),
"CREATE INDEX myidx ON mytable USING pilosa (i) WITH (async = false)",
"CREATE INDEX idx_i ON mytable USING pilosa (i) WITH (async = false)",
)
require.NoError(t, err)

_, _, err = e.Query(
newCtx(),
"CREATE INDEX myidx_multi ON mytable USING pilosa (i, s) WITH (async = false)",
"CREATE INDEX idx_s ON mytable USING pilosa (s) WITH (async = false)",
)
require.NoError(t, err)

_, _, err = e.Query(
newCtx(),
"CREATE INDEX idx_is ON mytable USING pilosa (i, s) WITH (async = false)",
)
require.NoError(t, err)

defer func() {
done, err := e.Catalog.DeleteIndex("mydb", "myidx", true)
done, err := e.Catalog.DeleteIndex("mydb", "idx_i", true)
require.NoError(t, err)
<-done

done, err = e.Catalog.DeleteIndex("foo", "myidx_multi", true)
done, err = e.Catalog.DeleteIndex("mydb", "idx_s", true)
require.NoError(t, err)
<-done

done, err = e.Catalog.DeleteIndex("foo", "idx_is", true)
require.NoError(t, err)
<-done
}()
Expand Down Expand Up @@ -1724,6 +1734,44 @@ func TestIndexes(t *testing.T) {
"SELECT * FROM mytable WHERE i = 1 AND i = 2",
([]sql.Row)(nil),
},
{
"SELECT i as mytable_i FROM mytable WHERE mytable_i = 2",
[]sql.Row{
{int64(2)},
},
},
{
"SELECT i as mytable_i FROM mytable WHERE mytable_i > 1",
[]sql.Row{
{int64(3)},
{int64(2)},
},
},
{
"SELECT i as mytable_i, s as mytable_s FROM mytable WHERE mytable_i = 2 AND mytable_s = 'second row'",
[]sql.Row{
{int64(2), "second row"},
},
},
{
"SELECT s, SUBSTRING(s, 1, 1) AS sub_s FROM mytable WHERE sub_s = 's'",
[]sql.Row{
{"second row", "s"},
},
},
{
"SELECT count(i) AS mytable_i, SUBSTR(s, -3) AS mytable_s FROM mytable WHERE i > 0 AND mytable_s='row' GROUP BY mytable_s",
[]sql.Row{
{int32(3), "row"},
},
},
{
"SELECT mytable_i FROM (SELECT i AS mytable_i FROM mytable) as t WHERE mytable_i > 1",
[]sql.Row{
{int64(2)},
{int64(3)},
},
},
}

for _, tt := range testCases {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ require (
github.com/mitchellh/hashstructure v1.0.0
github.com/oliveagle/jsonpath v0.0.0-20180606110733-2e52cf6e6852
github.com/opentracing/opentracing-go v1.0.2
github.com/pbnjay/memory v0.0.0-20190104145345-974d429e7ae4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is missed from the master

github.com/pelletier/go-toml v1.2.0 // indirect
github.com/pilosa/pilosa v1.2.0
github.com/pkg/errors v0.8.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ github.com/opentracing/opentracing-go v1.0.2 h1:3jA2P6O1F9UOrWVpwrIo17pu01KWvNWg
github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c h1:Lgl0gzECD8GnQ5QCWA8o6BtfL6mDH5rQgM4/fX3avOs=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pbnjay/memory v0.0.0-20190104145345-974d429e7ae4 h1:MfIUBZ1bz7TgvQLVa/yPJZOGeKEgs6eTKUjz3zB4B+U=
github.com/pbnjay/memory v0.0.0-20190104145345-974d429e7ae4/go.mod h1:RMU2gJXhratVxBDTFeOdNhd540tG57lt9FIUV0YLvIQ=
github.com/pelletier/go-toml v1.2.0 h1:T5zMGML61Wp+FlcbWjRDT7yAxhJNAiPPLOFECq181zc=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/pilosa/pilosa v1.2.0 h1:bi58fPsjyGIFnY5DtzECY+8RC9nRLvPrGC6fOqgtkTw=
Expand Down
100 changes: 76 additions & 24 deletions sql/analyzer/assign_indexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,42 @@ func assignIndexes(a *Analyzer, node sql.Node) (map[string]*indexLookup, error)
}
}()

var err error
aliases := make(map[string]sql.Expression)
var (
err error
fn func(node sql.Node) bool
)
fn = func(n sql.Node) bool {
if n == nil {
return true
}

if prj, ok := n.(*plan.Project); ok {
for _, ex := range prj.Expressions() {
if alias, ok := ex.(*expression.Alias); ok {
if _, ok := aliases[alias.Name()]; !ok {
aliases[alias.Name()] = alias.Child
}
}
}
} else {
for _, ch := range n.Children() {
plan.Inspect(ch, fn)
}
}

return true
}

plan.Inspect(node, func(node sql.Node) bool {
filter, ok := node.(*plan.Filter)
if !ok {
return true
}
fn(filter.Child)

var result map[string]*indexLookup
result, err = getIndexes(filter.Expression, a)
result, err = getIndexes(filter.Expression, aliases, a)
if err != nil {
return false
}
Expand All @@ -60,16 +87,16 @@ func assignIndexes(a *Analyzer, node sql.Node) (map[string]*indexLookup, error)
return indexes, err
}

func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error) {
func getIndexes(e sql.Expression, aliases map[string]sql.Expression, a *Analyzer) (map[string]*indexLookup, error) {
var result = make(map[string]*indexLookup)
switch e := e.(type) {
case *expression.Or:
leftIndexes, err := getIndexes(e.Left, a)
leftIndexes, err := getIndexes(e.Left, aliases, a)
if err != nil {
return nil, err
}

rightIndexes, err := getIndexes(e.Right, a)
rightIndexes, err := getIndexes(e.Right, aliases, a)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -101,7 +128,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
// the right branch is evaluable and the indexlookup supports set
// operations.
if !isEvaluable(c.Left()) && isEvaluable(c.Right()) {
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), c.Left())
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), unifyExpressions(aliases, c.Left())...)
if idx != nil {
var nidx sql.NegateIndex
if negate {
Expand Down Expand Up @@ -178,7 +205,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
*expression.GreaterThan,
*expression.LessThanOrEqual,
*expression.GreaterThanOrEqual:
idx, lookup, err := getComparisonIndex(a, e.(expression.Comparer))
idx, lookup, err := getComparisonIndex(a, e.(expression.Comparer), aliases)
if err != nil || lookup == nil {
return result, err
}
Expand All @@ -188,7 +215,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
lookup: lookup,
}
case *expression.Not:
r, err := getNegatedIndexes(a, e)
r, err := getNegatedIndexes(a, e, aliases)
if err != nil {
return nil, err
}
Expand All @@ -198,7 +225,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
}
case *expression.Between:
if !isEvaluable(e.Val) && isEvaluable(e.Upper) && isEvaluable(e.Lower) {
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), e.Val)
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), unifyExpressions(aliases, e.Val)...)
if idx != nil {
// release the index if it was not used
defer func() {
Expand Down Expand Up @@ -238,7 +265,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
exprs := splitExpression(e)
used := make(map[sql.Expression]struct{})

result, err := getMultiColumnIndexes(exprs, a, used)
result, err := getMultiColumnIndexes(exprs, a, used, aliases)
if err != nil {
return nil, err
}
Expand All @@ -248,7 +275,7 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
continue
}

indexes, err := getIndexes(e, a)
indexes, err := getIndexes(e, aliases, a)
if err != nil {
return nil, err
}
Expand All @@ -262,6 +289,28 @@ func getIndexes(e sql.Expression, a *Analyzer) (map[string]*indexLookup, error)
return result, nil
}

func unifyExpressions(aliases map[string]sql.Expression, expr ...sql.Expression) []sql.Expression {
expressions := make([]sql.Expression, len(expr))

for i, e := range expr {
uex := e
name := e.String()
if n, ok := e.(sql.Nameable); ok {
name = n.Name()
}

if aliases != nil && len(aliases) > 0 {
if alias, ok := aliases[name]; ok {
uex = alias
}
}

expressions[i] = uex
}

return expressions
}

func betweenIndexLookup(index sql.Index, upper, lower []interface{}) (sql.IndexLookup, error) {
ai, isAscend := index.(sql.AscendIndex)
di, isDescend := index.(sql.DescendIndex)
Expand Down Expand Up @@ -293,6 +342,7 @@ func betweenIndexLookup(index sql.Index, upper, lower []interface{}) (sql.IndexL
func getComparisonIndex(
a *Analyzer,
e expression.Comparer,
aliases map[string]sql.Expression,
) (sql.Index, sql.IndexLookup, error) {
left, right := e.Left(), e.Right()
// if the form is SOMETHING OP {INDEXABLE EXPR}, swap it, so it's {INDEXABLE EXPR} OP SOMETHING
Expand All @@ -301,7 +351,7 @@ func getComparisonIndex(
}

if !isEvaluable(left) && isEvaluable(right) {
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), left)
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), unifyExpressions(aliases, left)...)
if idx != nil {
value, err := right.Eval(sql.NewEmptyContext(), nil)
if err != nil {
Expand Down Expand Up @@ -363,10 +413,10 @@ func comparisonIndexLookup(
return nil, nil
}

func getNegatedIndexes(a *Analyzer, not *expression.Not) (map[string]*indexLookup, error) {
func getNegatedIndexes(a *Analyzer, not *expression.Not, aliases map[string]sql.Expression) (map[string]*indexLookup, error) {
switch e := not.Child.(type) {
case *expression.Not:
return getIndexes(e.Child, a)
return getIndexes(e.Child, aliases, a)
case *expression.Equals:
left, right := e.Left(), e.Right()
// if the form is SOMETHING OP {INDEXABLE EXPR}, swap it, so it's {INDEXABLE EXPR} OP SOMETHING
Expand All @@ -378,7 +428,7 @@ func getNegatedIndexes(a *Analyzer, not *expression.Not) (map[string]*indexLooku
return nil, nil
}

idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), left)
idx := a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), unifyExpressions(aliases, left)...)
if idx == nil {
return nil, nil
}
Expand Down Expand Up @@ -410,37 +460,37 @@ func getNegatedIndexes(a *Analyzer, not *expression.Not) (map[string]*indexLooku
return result, nil
case *expression.GreaterThan:
lte := expression.NewLessThanOrEqual(e.Left(), e.Right())
return getIndexes(lte, a)
return getIndexes(lte, aliases, a)
case *expression.GreaterThanOrEqual:
lt := expression.NewLessThan(e.Left(), e.Right())
return getIndexes(lt, a)
return getIndexes(lt, aliases, a)
case *expression.LessThan:
gte := expression.NewGreaterThanOrEqual(e.Left(), e.Right())
return getIndexes(gte, a)
return getIndexes(gte, aliases, a)
case *expression.LessThanOrEqual:
gt := expression.NewGreaterThan(e.Left(), e.Right())
return getIndexes(gt, a)
return getIndexes(gt, aliases, a)
case *expression.Between:
or := expression.NewOr(
expression.NewLessThan(e.Val, e.Lower),
expression.NewGreaterThan(e.Val, e.Upper),
)

return getIndexes(or, a)
return getIndexes(or, aliases, a)
case *expression.Or:
and := expression.NewAnd(
expression.NewNot(e.Left),
expression.NewNot(e.Right),
)

return getIndexes(and, a)
return getIndexes(and, aliases, a)
case *expression.And:
or := expression.NewOr(
expression.NewNot(e.Left),
expression.NewNot(e.Right),
)

return getIndexes(or, a)
return getIndexes(or, aliases, a)
default:
return nil, nil

Expand Down Expand Up @@ -481,6 +531,7 @@ func getMultiColumnIndexes(
exprs []sql.Expression,
a *Analyzer,
used map[sql.Expression]struct{},
aliases map[string]sql.Expression,
) (map[string]*indexLookup, error) {
result := make(map[string]*indexLookup)
columnExprs := columnExprsByTable(exprs)
Expand All @@ -502,7 +553,7 @@ func getMultiColumnIndexes(
}

if len(selected) > 0 {
index, lookup, err := getMultiColumnIndexForExpressions(a, selected, exps, used)
index, lookup, err := getMultiColumnIndexForExpressions(a, selected, exps, used, aliases)
if err != nil || lookup == nil {
if index != nil {
a.Catalog.ReleaseIndex(index)
Expand Down Expand Up @@ -534,8 +585,9 @@ func getMultiColumnIndexForExpressions(
selected []sql.Expression,
exprs []columnExpr,
used map[sql.Expression]struct{},
aliases map[string]sql.Expression,
) (index sql.Index, lookup sql.IndexLookup, err error) {
index = a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), selected...)
index = a.Catalog.IndexByExpression(a.Catalog.CurrentDatabase(), unifyExpressions(aliases, selected...)...)
if index != nil {
var first sql.Expression
for _, e := range exprs {
Expand Down
4 changes: 2 additions & 2 deletions sql/analyzer/assign_indexes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ func TestGetIndexes(t *testing.T) {
t.Run(tt.expr.String(), func(t *testing.T) {
require := require.New(t)

result, err := getIndexes(tt.expr, a)
result, err := getIndexes(tt.expr, nil, a)
if tt.ok {
require.NoError(err)
require.Equal(tt.expected, result)
Expand Down Expand Up @@ -779,7 +779,7 @@ func TestGetMultiColumnIndexes(t *testing.T) {
lit(6),
),
}
result, err := getMultiColumnIndexes(exprs, a, used)
result, err := getMultiColumnIndexes(exprs, a, used, nil)
require.NoError(err)

expected := map[string]*indexLookup{
Expand Down