Skip to content

Commit

Permalink
Merge pull request #274 from dolthub/zachmu/index-bug
Browse files Browse the repository at this point in the history 
Fixed various bugs in subquery execution, added table ordering optimization
  • Loading branch information
@zachmu
zachmu authored Feb 1, 2021
2 parents cc04b53 + 1d6e367 commit aaac5f2
Show file tree
Hide file tree
Showing 33 changed files with 1,198 additions and 678 deletions.
2 changes: 1 addition & 1 deletion enginetest/insert_queries.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ var InsertQueries = []WriteQueryTest{
WriteQuery: "INSERT INTO mytable (i,s) values (1,'mar'), (2,'par') ON DUPLICATE KEY UPDATE s=CONCAT(VALUES(s), 'tial')",
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(4)}},
SelectQuery: "SELECT * FROM mytable WHERE i IN (1,2) ORDER BY i",
ExpectedSelect: []sql.Row{{int64(1), "martial"},{int64(2), "partial"}},
ExpectedSelect: []sql.Row{{int64(1), "martial"}, {int64(2), "partial"}},
},
{
WriteQuery: "INSERT INTO mytable (i,s) values (1,'maybe') ON DUPLICATE KEY UPDATE i=VALUES(i)+8000, s=VALUES(s)",
Expand Down
7 changes: 3 additions & 4 deletions enginetest/memory_engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,12 @@ func TestSingleQuery(t *testing.T) {

var test enginetest.QueryTest
test = enginetest.QueryTest{
Query: `SELECT * FROM mytable mt INNER JOIN othertable ot ON mt.i = ot.i2 AND mt.i > 2`,
Query: "SELECT (select s from mytable mt where sub.i = mt.i) as subi FROM (select s,i,'hello' FROM mytable where i = 1) as sub;",
Expected: []sql.Row{
{int64(3), "third row", "first", int64(3)},
{"first row"},
},
}

fmt.Sprintf("%v", test)
fmt.Sprintf("%v", test)

harness := enginetest.NewMemoryHarness("", 1, testNumPartitions, true, mergableIndexDriver)
engine := enginetest.NewEngine(t, harness)
Expand Down
44 changes: 44 additions & 0 deletions enginetest/queries.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,36 @@ var QueryTests = []QueryTest{
{"second row", int64(2)},
{"third row", int64(3)}},
},
{
Query: "SELECT s, (select i from mytable mt where sub.i = mt.i) as subi FROM (select i,s,'hello' FROM mytable where s = 'first row') as sub;",
Expected: []sql.Row{
{"first row", int64(1)},
},
},
{
Query: "SELECT (select s from mytable mt where sub.i = mt.i) as subi FROM (select i,s,'hello' FROM mytable where i = 1) as sub;",
Expected: []sql.Row{
{"first row"},
},
},
{
Query: "SELECT (select s from mytable mt where sub.i = mt.i) as subi FROM (select s,i,'hello' FROM mytable where i = 1) as sub;",
Expected: []sql.Row{
{"first row"},
},
},
{
Query: "SELECT s, (select i from mytable mt where sub.i = mt.i) as subi FROM (select 'hello',i,s FROM mytable where s = 'first row') as sub;",
Expected: []sql.Row{
{"first row", int64(1)},
},
},
{
Query: "SELECT (select s from mytable mt where sub.i = mt.i) as subi FROM (select 'hello',i,s FROM mytable where i = 1) as sub;",
Expected: []sql.Row{
{"first row"},
},
},
{
Query: "SELECT s,i FROM MyTable ORDER BY 2",
Expected: []sql.Row{
Expand Down Expand Up @@ -458,6 +488,10 @@ var QueryTests = []QueryTest{
Query: "SELECT i FROM mytable WHERE i > 2",
Expected: []sql.Row{{int64(3)}},
},
{
Query: "SELECT i FROM mytable WHERE i+1 > 3",
Expected: []sql.Row{{int64(3)}},
},
{
Query: "SELECT i FROM mytable WHERE i < 2",
Expected: []sql.Row{{int64(1)}},
Expand Down Expand Up @@ -887,6 +921,16 @@ var QueryTests = []QueryTest{
{"first"},
},
},
{
Query: "SELECT t1.i FROM mytable t1 JOIN mytable t2 on t1.i = t2.i + 1 where t1.i = 2 and t2.i = 1",
Expected: []sql.Row{
{2},
},
},
{
Query: "SELECT t1.i FROM mytable t1 JOIN mytable t2 on t1.i = t2.i + 1 where t1.i = 2 and t2.i = 3",
Expected: []sql.Row{},
},
{
Query: "SELECT i, i2, s2 FROM mytable INNER JOIN othertable ON i = i2 ORDER BY i",
Expected: []sql.Row{
Expand Down
67 changes: 41 additions & 26 deletions enginetest/query_plans.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ type QueryPlanTest struct {
// other features. These tests are fragile because they rely on string representations of query plans, but they're much
// easier to construct this way.
var PlanTests = []QueryPlanTest{
{
Query: "SELECT t1.i FROM mytable t1 JOIN mytable t2 on t1.i = t2.i + 1 where t1.i = 2 and t2.i = 1",
ExpectedPlan: "Project(t1.i)\n" +
" └─ IndexedJoin(t1.i = t2.i + 1)\n" +
" ├─ Filter(t2.i = 1)\n" +
" │ └─ TableAlias(t2)\n" +
" │ └─ IndexedTableAccess(mytable on [mytable.i])\n" +
" └─ Filter(t1.i = 2)\n" +
" └─ TableAlias(t1)\n" +
" └─ IndexedTableAccess(mytable on [mytable.i])\n" +
"",
},
{
Query: "SELECT i, i2, s2 FROM mytable INNER JOIN othertable ON i = i2",
ExpectedPlan: "Project(mytable.i, othertable.i2, othertable.s2)\n" +
Expand Down Expand Up @@ -92,8 +104,7 @@ var PlanTests = []QueryPlanTest{
ExpectedPlan: "IndexedJoin(mt.i = ot.i2)\n" +
" ├─ Filter(mt.i > 2)\n" +
" │ └─ TableAlias(mt)\n" +
" │ └─ Indexed table access on index [mytable.i]\n" +
" │ └─ Table(mytable)\n" +
" │ └─ IndexedTableAccess(mytable on [mytable.i])\n" +
" └─ TableAlias(ot)\n" +
" └─ IndexedTableAccess(othertable on [othertable.i2])\n" +
"",
Expand Down Expand Up @@ -349,9 +360,8 @@ var PlanTests = []QueryPlanTest{
Query: "SELECT pk,i,f FROM one_pk LEFT JOIN niltable ON pk=i WHERE pk > 1",
ExpectedPlan: "Project(one_pk.pk, niltable.i, niltable.f)\n" +
" └─ LeftIndexedJoin(one_pk.pk = niltable.i)\n" +
" ├─ Indexed table access on index [one_pk.pk]\n" +
" │ └─ Filter(one_pk.pk > 1)\n" +
" │ └─ Table(one_pk)\n" +
" ├─ Filter(one_pk.pk > 1)\n" +
" │ └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" └─ IndexedTableAccess(niltable on [niltable.i])\n" +
"",
},
Expand Down Expand Up @@ -520,10 +530,10 @@ var PlanTests = []QueryPlanTest{
ExpectedPlan: "Sort(one_pk.pk ASC)\n" +
" └─ Project(one_pk.pk, niltable.i, niltable.f)\n" +
" └─ LeftIndexedJoin(one_pk.pk = niltable.i)\n" +
" ├─ Indexed table access on index [one_pk.pk]\n" +
" │ └─ Filter(one_pk.pk > 1)\n" +
" │ └─ Table(one_pk)\n" +
" └─ IndexedTableAccess(niltable on [niltable.i])\n",
" ├─ Filter(one_pk.pk > 1)\n" +
" │ └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" └─ IndexedTableAccess(niltable on [niltable.i])\n" +
"",
},
{
Query: "SELECT pk,i,f FROM one_pk RIGHT JOIN niltable ON pk=i ORDER BY 2,3",
Expand Down Expand Up @@ -674,22 +684,33 @@ var PlanTests = []QueryPlanTest{
" └─ CrossJoin\n" +
" ├─ Filter(t1.pk = 1)\n" +
" │ └─ TableAlias(t1)\n" +
" │ └─ Indexed table access on index [one_pk.pk]\n" +
" │ └─ Table(one_pk)\n" +
" │ └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" └─ Filter(t2.pk2 = 1)\n" +
" └─ TableAlias(t2)\n" +
" └─ Table(two_pk)\n" +
"",
},
{
// TODO: This should use an index for two_pk as well
Query: "SELECT pk,pk1,pk2 FROM one_pk t1, two_pk t2 WHERE pk=1 AND pk2=1 AND pk1=1 ORDER BY 1,2",
ExpectedPlan: "Sort(t1.pk ASC, t2.pk1 ASC)\n" +
" └─ Project(t1.pk, t2.pk1, t2.pk2)\n" +
" └─ CrossJoin\n" +
" ├─ Filter(t1.pk = 1)\n" +
" │ └─ TableAlias(t1)\n" +
" │ └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" └─ Filter(t2.pk2 = 1 AND t2.pk1 = 1)\n" +
" └─ TableAlias(t2)\n" +
" └─ Table(two_pk)\n",
},
{
Query: `SELECT i FROM mytable mt
WHERE (SELECT i FROM mytable where i = mt.i and i > 2) IS NOT NULL
AND (SELECT i2 FROM othertable where i2 = i) IS NOT NULL`,
ExpectedPlan: "Project(mt.i)\n" +
" └─ Filter(NOT((Project(mytable.i)\n" +
" └─ Filter(mytable.i = mt.i AND mytable.i > 2)\n" +
" └─ Indexed table access on index [mytable.i]\n" +
" └─ Table(mytable)\n" +
" └─ IndexedTableAccess(mytable on [mytable.i])\n" +
" ) IS NULL) AND NOT((Project(othertable.i2)\n" +
" └─ Filter(othertable.i2 = mt.i)\n" +
" └─ IndexedTableAccess(othertable on [othertable.i2])\n" +
Expand Down Expand Up @@ -719,15 +740,13 @@ var PlanTests = []QueryPlanTest{
ExpectedPlan: "Sort(t1.pk ASC, t2.pk2 ASC)\n" +
" └─ Project(t1.pk, t2.pk2, (Limit(1)\n" +
" └─ Project(one_pk.pk)\n" +
" └─ Indexed table access on index [one_pk.pk]\n" +
" └─ Filter(one_pk.pk = 1)\n" +
" └─ Table(one_pk)\n" +
" └─ Filter(one_pk.pk = 1)\n" +
" └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" ))\n" +
" └─ CrossJoin\n" +
" ├─ Filter(t1.pk = 1)\n" +
" │ └─ TableAlias(t1)\n" +
" │ └─ Indexed table access on index [one_pk.pk]\n" +
" │ └─ Table(one_pk)\n" +
" │ └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
" └─ Filter(t2.pk2 = 1)\n" +
" └─ TableAlias(t2)\n" +
" └─ Table(two_pk)\n" +
Expand All @@ -743,10 +762,8 @@ var PlanTests = []QueryPlanTest{
{
Query: "DELETE FROM two_pk WHERE pk1 = 1 AND pk2 = 2",
ExpectedPlan: "Delete\n" +
" └─ Indexed table access on index [two_pk.pk1,two_pk.pk2]\n" +
" └─ Filter(two_pk.pk1 = 1 AND two_pk.pk2 = 2)\n" +
" └─ Table(two_pk)\n" +
"",
" └─ Filter(two_pk.pk1 = 1 AND two_pk.pk2 = 2)\n" +
" └─ IndexedTableAccess(two_pk on [two_pk.pk1,two_pk.pk2])\n",
},
{
Query: "UPDATE two_pk SET c1 = 1 WHERE c1 > 1",
Expand All @@ -760,9 +777,7 @@ var PlanTests = []QueryPlanTest{
Query: "UPDATE two_pk SET c1 = 1 WHERE pk1 = 1 AND pk2 = 2",
ExpectedPlan: "Update\n" +
" └─ UpdateSource(SET two_pk.c1 = 1)\n" +
" └─ Indexed table access on index [two_pk.pk1,two_pk.pk2]\n" +
" └─ Filter(two_pk.pk1 = 1 AND two_pk.pk2 = 2)\n" +
" └─ Table(two_pk)\n" +
"",
" └─ Filter(two_pk.pk1 = 1 AND two_pk.pk2 = 2)\n" +
" └─ IndexedTableAccess(two_pk on [two_pk.pk1,two_pk.pk2])\n",
},
}
49 changes: 49 additions & 0 deletions enginetest/trigger_queries.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,31 @@ var TriggerTests = []ScriptTest{
{32, 41},
},
},
{
Name: "trigger before update, with indexed update",
SetUpScript: []string{
"create table a (x int primary key, y int, unique key (y))",
"create table b (z int primary key)",
"insert into a values (1,3), (10,20)",
"create trigger insert_b before update on a for each row insert into b values (old.x * 10)",
"update a set x = x + 1 where y = 20",
},
Assertions: []ScriptTestAssertion{
{
Query: "select x, y from a order by 1",
Expected: []sql.Row{
{1, 3},
{11, 20},
},
},
{
Query: "select z from b",
Expected: []sql.Row{
{100},
},
},
},
},
// DELETE triggers
{
Name: "trigger after delete, insert into other table",
Expand Down Expand Up @@ -660,6 +685,30 @@ var TriggerTests = []ScriptTest{
},
},
},
{
Name: "trigger before delete, delete with index",
SetUpScript: []string{
"create table a (x int primary key, z int, unique key (z))",
"create table b (y int primary key)",
"insert into a values (0,1), (2,3), (4,5)",
"create trigger insert_b before delete on a for each row insert into b values (old.x * 2)",
"delete from a where z > 2",
},
Assertions: []ScriptTestAssertion{
{
Query: "select x from a order by 1",
Expected: []sql.Row{
{0},
},
},
{
Query: "select y from b order by 1",
Expected: []sql.Row{
{4}, {8},
},
},
},
},
// Multiple triggers defined
{
Name: "triggers before and after insert",
Expand Down
4 changes: 2 additions & 2 deletions sql/analyzer/analyzer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,9 @@ func (a *Analyzer) LogNode(n sql.Node) {
if a != nil && n != nil && a.Verbose {
if len(a.contextStack) > 0 {
ctx := strings.Join(a.contextStack, "/")
fmt.Printf("%s:\n%s", ctx, sql.DebugString(n))
logrus.Infof("%s:\n%s", ctx, sql.DebugString(n))
} else {
fmt.Printf("%s", sql.DebugString(n))
logrus.Infof("%s", sql.DebugString(n))
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions sql/analyzer/analyzer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestAnalyzer_Analyze(t *testing.T) {
plan.NewUnresolvedTable("mytable", ""),
)
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
var expected sql.Node = plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i]", plan.NewResolvedTable(
var expected sql.Node = plan.NewDecoratedNode("Projected table access on [i]", plan.NewResolvedTable(
table.WithProjection([]string{"i"}),
))
require.NoError(err)
Expand All @@ -89,7 +89,7 @@ func TestAnalyzer_Analyze(t *testing.T) {
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
require.NoError(err)

expected = plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "t"})))
expected = plan.NewDecoratedNode("Projected table access on [i t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "t"})))
assertNodesEqualWithDiff(t, expected, analyzed)

notAnalyzed = plan.NewProject(
Expand All @@ -102,7 +102,7 @@ func TestAnalyzer_Analyze(t *testing.T) {
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
require.NoError(err)

expected = plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "t"})))
expected = plan.NewDecoratedNode("Projected table access on [i t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "t"})))
assertNodesEqualWithDiff(t, expected, analyzed)

notAnalyzed = plan.NewProject(
Expand All @@ -116,7 +116,7 @@ func TestAnalyzer_Analyze(t *testing.T) {
[]sql.Expression{
expression.NewAlias("foo", expression.NewGetFieldWithTable(0, sql.Int32, "mytable", "i", false)),
},
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i]",
plan.NewDecoratedNode("Projected table access on [i]",
plan.NewResolvedTable(table.WithProjection([]string{"i"}))),
)
require.NoError(err)
Expand All @@ -133,8 +133,8 @@ func TestAnalyzer_Analyze(t *testing.T) {
),
)
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
expected = plan.NewDecoratedNode(plan.DecorationTypeFilteredAccess, "Filtered table access on [mytable.i = 1]",
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i]",
expected = plan.NewDecoratedNode("Filtered table access on [mytable.i = 1]",
plan.NewDecoratedNode("Projected table access on [i]",
plan.NewResolvedTable(
table.WithFilters([]sql.Expression{
expression.NewEquals(
Expand All @@ -160,8 +160,8 @@ func TestAnalyzer_Analyze(t *testing.T) {
)
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
expected = plan.NewCrossJoin(
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i]", plan.NewResolvedTable(table.WithProjection([]string{"i"}))),
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i2]", plan.NewResolvedTable(table2.WithProjection([]string{"i2"}))),
plan.NewDecoratedNode("Projected table access on [i]", plan.NewResolvedTable(table.WithProjection([]string{"i"}))),
plan.NewDecoratedNode("Projected table access on [i2]", plan.NewResolvedTable(table2.WithProjection([]string{"i2"}))),
)
require.NoError(err)
assertNodesEqualWithDiff(t, expected, analyzed)
Expand All @@ -177,7 +177,7 @@ func TestAnalyzer_Analyze(t *testing.T) {
analyzed, err = a.Analyze(ctx, notAnalyzed, nil)
expected = plan.NewLimit(
int64(1),
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i]",
plan.NewDecoratedNode("Projected table access on [i]",
plan.NewResolvedTable(table.WithProjection([]string{"i"}))),
)
require.NoError(err)
Expand Down Expand Up @@ -380,14 +380,14 @@ func TestMixInnerAndNaturalJoins(t *testing.T) {
},
plan.NewInnerJoin(
plan.NewInnerJoin(
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [i f t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "f", "t"}))),
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [f2 i2 t2]", plan.NewResolvedTable(table2.WithProjection([]string{"f2", "i2", "t2"}))),
plan.NewDecoratedNode("Projected table access on [i f t]", plan.NewResolvedTable(table.WithProjection([]string{"i", "f", "t"}))),
plan.NewDecoratedNode("Projected table access on [f2 i2 t2]", plan.NewResolvedTable(table2.WithProjection([]string{"f2", "i2", "t2"}))),
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int32, "mytable", "i", false),
expression.NewGetFieldWithTable(4, sql.Int32, "mytable2", "i2", false),
),
),
plan.NewDecoratedNode(plan.DecorationTypeProjectedAccess, "Projected table access on [t3 i f2]", plan.NewResolvedTable(table3.WithProjection([]string{"t3", "i", "f2"}))),
plan.NewDecoratedNode("Projected table access on [t3 i f2]", plan.NewResolvedTable(table3.WithProjection([]string{"t3", "i", "f2"}))),
expression.NewAnd(
expression.NewEquals(
expression.NewGetFieldWithTable(0, sql.Int32, "mytable", "i", false),
Expand Down
Loading

0 comments on commit aaac5f2

Please sign in to comment.