Skip to content

Commit db5f2dc

Browse files
committed
Fixed bug in cross joins involving disjunctions that contain more than one table.
Signed-off-by: Zach Musgrave <zach@liquidata.co>
1 parent 4323333 commit db5f2dc

File tree

2 files changed

+143
-16
lines changed

2 files changed

+143
-16
lines changed

engine_test.go

Lines changed: 115 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type queryTest struct {
3030
expected []sql.Row
3131
}
3232

33-
var queries = []queryTest {
33+
var queries = []queryTest{
3434
{
3535
"SELECT i FROM mytable;",
3636
[]sql.Row{{int64(1)}, {int64(2)}, {int64(3)}},
@@ -49,20 +49,6 @@ var queries = []queryTest {
4949
{"second row", int64(2)},
5050
{"third row", int64(3)}},
5151
},
52-
{
53-
"select pk,pk1,pk2 from one_pk,two_pk where pk=0 and pk1=0 or pk2=1 order by 1,2,3",
54-
[]sql.Row{
55-
{0,0,0},
56-
{0,0,1},
57-
{0,1,1},
58-
{1,0,1},
59-
{1,1,1},
60-
{2,0,1},
61-
{2,1,1},
62-
{3,0,1},
63-
{3,1,1},
64-
},
65-
},
6652
{
6753
"SELECT i + 1 FROM mytable;",
6854
[]sql.Row{{int64(2)}, {int64(3)}, {int64(4)}},
@@ -367,7 +353,7 @@ var queries = []queryTest {
367353
},
368354
{
369355
"SELECT s FROM mytable INNER JOIN othertable " +
370-
"ON substring(s2, 1, 2) != '' AND i = i2",
356+
"ON substring(s2, 1, 2) != '' AND i = i2",
371357
[]sql.Row{
372358
{"first row"},
373359
{"second row"},
@@ -1354,6 +1340,119 @@ var queries = []queryTest {
13541340
{int64(4)},
13551341
},
13561342
},
1343+
{
1344+
"select pk,pk1,pk2 from one_pk, two_pk order by 1,2,3",
1345+
[]sql.Row{
1346+
{0, 0, 0},
1347+
{0, 0, 1},
1348+
{0, 1, 0},
1349+
{0, 1, 1},
1350+
{1, 0, 0},
1351+
{1, 0, 1},
1352+
{1, 1, 0},
1353+
{1, 1, 1},
1354+
{2, 0, 0},
1355+
{2, 0, 1},
1356+
{2, 1, 0},
1357+
{2, 1, 1},
1358+
{3, 0, 0},
1359+
{3, 0, 1},
1360+
{3, 1, 0},
1361+
{3, 1, 1},
1362+
},
1363+
},
1364+
{
1365+
"select t1.c1,t2.c2 from one_pk t1, two_pk t2 where pk1=1 and pk2=1 order by 1,2",
1366+
[]sql.Row{
1367+
{0, 30},
1368+
{10, 30},
1369+
{20, 30},
1370+
{30, 30},
1371+
},
1372+
},
1373+
{
1374+
"select t1.c1,t2.c2 from one_pk t1, two_pk t2 where pk1=1 or pk2=1 order by 1,2",
1375+
[]sql.Row{
1376+
{0, 10},
1377+
{0, 20},
1378+
{0, 30},
1379+
{10, 10},
1380+
{10, 20},
1381+
{10, 30},
1382+
{20, 10},
1383+
{20, 20},
1384+
{20, 30},
1385+
{30, 10},
1386+
{30, 20},
1387+
{30, 30},
1388+
},
1389+
},
1390+
{
1391+
"select pk,pk2 from one_pk t1, two_pk t2 where pk=1 and pk2=1 order by 1,2",
1392+
[]sql.Row{
1393+
{1, 1},
1394+
{1, 1},
1395+
},
1396+
},
1397+
{
1398+
"select pk,pk1,pk2 from one_pk,two_pk where pk=0 and pk1=0 or pk2=1 order by 1,2,3",
1399+
[]sql.Row{
1400+
{0, 0, 0},
1401+
{0, 0, 1},
1402+
{0, 1, 1},
1403+
{1, 0, 1},
1404+
{1, 1, 1},
1405+
{2, 0, 1},
1406+
{2, 1, 1},
1407+
{3, 0, 1},
1408+
{3, 1, 1},
1409+
},
1410+
},
1411+
{
1412+
"select pk,pk1,pk2 from one_pk,two_pk where one_pk.c1=two_pk.c1 order by 1,2,3",
1413+
[]sql.Row{
1414+
{0, 0, 0},
1415+
{1, 0, 1},
1416+
{2, 1, 0},
1417+
{3, 1, 1},
1418+
},
1419+
},
1420+
{
1421+
"select one_pk.c5,pk1,pk2 from one_pk,two_pk where pk=pk1 order by 1,2,3",
1422+
[]sql.Row{
1423+
{0, 0, 0},
1424+
{0, 0, 1},
1425+
{10, 1, 0},
1426+
{10, 1, 1},
1427+
},
1428+
},
1429+
{
1430+
"select pk,pk1,pk2 from one_pk join two_pk on one_pk.c1=two_pk.c1 where pk=1 order by 1,2,3",
1431+
[]sql.Row{
1432+
{1, 0, 1},
1433+
},
1434+
},
1435+
{
1436+
"select pk,pk1,pk2,one_pk.c1 as foo, two_pk.c1 as bar from one_pk join two_pk on one_pk.c1=two_pk.c1 order by 1,2,3",
1437+
[]sql.Row{
1438+
{0, 0, 0, 0, 0},
1439+
{1, 0, 1, 10, 10},
1440+
{2, 1, 0, 20, 20},
1441+
{3, 1, 1, 30, 30},
1442+
},
1443+
},
1444+
{
1445+
"select pk,pk1,pk2,one_pk.c1 as foo,two_pk.c1 as bar from one_pk join two_pk on one_pk.c1=two_pk.c1 where one_pk.c1=10",
1446+
[]sql.Row{
1447+
{1, 0, 1, 10, 10},
1448+
},
1449+
},
1450+
{
1451+
"select pk,pk1,pk2 from one_pk join two_pk on pk1-pk>0 and pk2<1",
1452+
[]sql.Row{
1453+
{0, 1, 0},
1454+
},
1455+
},
13571456
}
13581457

13591458
var infoSchemaQueries = []queryTest {

sql/analyzer/assign_indexes.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ func getIndexes(e sql.Expression, aliases map[string]sql.Expression, a *Analyzer
9191
var result = make(map[string]*indexLookup)
9292
switch e := e.(type) {
9393
case *expression.Or:
94+
// If more than one table is involved in a disjunction, we can't use indexed lookups. This is because we will
95+
// inappropriately restrict the iterated values of the indexed table to matching index values, when during a cross
96+
// join we must consider every row from each table.
97+
if len(findTables(e)) > 1 {
98+
return nil, nil
99+
}
100+
94101
leftIndexes, err := getIndexes(e.Left, aliases, a)
95102
if err != nil {
96103
return nil, err
@@ -299,6 +306,27 @@ func getIndexes(e sql.Expression, aliases map[string]sql.Expression, a *Analyzer
299306
return result, nil
300307
}
301308

309+
// Returns the tables used in the expression given
310+
func findTables(e sql.Expression) []string {
311+
tables := make(map[string]bool)
312+
expression.Inspect(e, func(e sql.Expression) bool {
313+
switch e := e.(type) {
314+
case *expression.GetField:
315+
tables[e.Table()] = true
316+
return false
317+
default:
318+
return true
319+
}
320+
})
321+
322+
var names []string
323+
for table := range tables {
324+
names = append(names, table)
325+
}
326+
327+
return names
328+
}
329+
302330
func unifyExpressions(aliases map[string]sql.Expression, expr ...sql.Expression) []sql.Expression {
303331
expressions := make([]sql.Expression, len(expr))
304332

0 commit comments

Comments
 (0)