Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Right/left/Cross/Indexed Joins and Implement Table Edit Accumulator #596

Merged
merged 25 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
11840a3
merge indexed join stuff
VinaiRachakonda Oct 18, 2021
71e2916
working prototype for the indexed join
VinaiRachakonda Oct 18, 2021
7d5a573
some cleanup
VinaiRachakonda Oct 18, 2021
798d71f
uncomment
VinaiRachakonda Oct 19, 2021
9c86eb8
Merge branch 'main' into vinai/additional-update-join-support
VinaiRachakonda Oct 19, 2021
322bd67
fix the sorter analyzer step. Need to get memory table editor to work
VinaiRachakonda Oct 19, 2021
810003a
really stupid implementation of an edit accumulator
VinaiRachakonda Oct 20, 2021
86cb384
basic implementation
VinaiRachakonda Oct 20, 2021
822267e
rewrote editor to get all tests passing for now:
VinaiRachakonda Oct 21, 2021
b722d9e
tests pass with split
VinaiRachakonda Oct 21, 2021
f0f9648
formatting and still passing tests
VinaiRachakonda Oct 21, 2021
bf3d11c
cleanup and undo some dumd goland refactors
VinaiRachakonda Oct 21, 2021
f92cbce
cleanup/remove flakiness
VinaiRachakonda Oct 21, 2021
b8b1337
formatting and cleaning up sloppy code
VinaiRachakonda Oct 21, 2021
00c0ecd
cleanup edits interface
VinaiRachakonda Oct 22, 2021
08a2262
clean up interface
VinaiRachakonda Oct 22, 2021
ba837b8
realized bug is that table needs to be an implicity sorted map
VinaiRachakonda Oct 22, 2021
0c2e19b
this is sorting logic. Checking it in for now but not useful in the l…
VinaiRachakonda Oct 22, 2021
db56f4e
removed sorting implementaiton and fixed the flaky test
VinaiRachakonda Oct 22, 2021
15d8a24
merge
VinaiRachakonda Oct 22, 2021
6bbf376
final nit
VinaiRachakonda Oct 22, 2021
0600ab2
working solution, needs more tests
VinaiRachakonda Oct 23, 2021
d4e7fea
more tests
VinaiRachakonda Oct 25, 2021
5358690
cleanup and revert single query
VinaiRachakonda Oct 25, 2021
55baecc
Pr review comments
VinaiRachakonda Oct 26, 2021
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
10 changes: 5 additions & 5 deletions enginetest/enginetests.go
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ func TestWindowAgg(t *testing.T, harness Harness) {
{5, 0},
}, nil, nil)

TestQuery(t, harness, e, `SELECT a, first_value(a) over (partition by b) FROM t1 order by a`, []sql.Row{
TestQuery(t, harness, e, `SELECT a, first_value(a) over (partition by b order by a ASC, c ASC) FROM t1 order by a`, []sql.Row{
{0, 0},
{1, 1},
{2, 2},
Expand All @@ -2465,12 +2465,12 @@ func TestWindowAgg(t *testing.T, harness Harness) {
{5, 5},
}, nil, nil)

TestQuery(t, harness, e, `SELECT a, first_value(a-1) over (partition by b order by c) FROM t1 order by a`, []sql.Row{
TestQuery(t, harness, e, `SELECT a, first_value(a-1) over (partition by b order by a ASC, c ASC) FROM t1 order by a`, []sql.Row{
{0, -1},
{1, 3},
{1, 0},
{2, 1},
{3, -1},
{4, 3},
{4, 0},
{5, 4},
}, nil, nil)

Expand Down Expand Up @@ -2876,7 +2876,7 @@ func TestSessionSelectLimit(t *testing.T, harness Harness) {
},
},
{
Query: "SELECT i FROM (SELECT i FROM mytable LIMIT 2) t ORDER BY i",
Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i LIMIT 2) t",
Expected: []sql.Row{{int64(1)}},
},
// TODO: this is broken: the session limit is applying inappropriately to the subquery
Expand Down
23 changes: 23 additions & 0 deletions enginetest/insert_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,29 @@ var InsertScripts = []ScriptTest{
},
},
},
{
Name: "INSERT Accumulator tests",
SetUpScript: []string{
"CREATE TABLE test(pk int primary key, val int)",
"INSERT INTO test values (1,1)",
},
Assertions: []ScriptTestAssertion{
{
Query: `INSERT INTO test VALUES (2,2),(2,3)`,
ExpectedErr: sql.ErrPrimaryKeyViolation,
},
{
Query: `DELETE FROM test where pk = 1;`,
Expected: []sql.Row{{sql.OkResult{RowsAffected: 1}}},
},
{
Query: `INSERT INTO test VALUES (1,1)`,
Expected: []sql.Row{
{sql.OkResult{RowsAffected: 1}},
},
},
},
},
}

var InsertErrorTests = []GenericErrorQueryTest{
Expand Down
27 changes: 20 additions & 7 deletions enginetest/memory_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"bufio"
"fmt"
"io/ioutil"
"math"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -121,14 +120,28 @@ func TestSingleScript(t *testing.T) {

var scripts = []enginetest.ScriptTest{
{
Name: "set system variable defaults",
Name: "failed statements data validation for INSERT, UPDATE",
SetUpScript: []string{
"set @@auto_increment_increment = 100, sql_select_limit = 1",
"set @@auto_increment_increment = default, sql_select_limit = default",
"CREATE TABLE test (pk BIGINT PRIMARY KEY, v1 BIGINT, INDEX (v1));",
"INSERT INTO test VALUES (1,1), (5,5), (4,4);",
},
Query: "SELECT @@auto_increment_increment, @@sql_select_limit",
Expected: []sql.Row{
{1, math.MaxInt32},
Assertions: []enginetest.ScriptTestAssertion{
{
Query: "INSERT INTO test VALUES (2,2), (3,3), (1,1);",
ExpectedErrStr: "duplicate primary key given: [1]",
},
{
Query: "SELECT * FROM test;",
Expected: []sql.Row{{1, 1}, {4, 4}, {5, 5}},
},
{
Query: "UPDATE test SET pk = pk + 1 ORDER BY pk;",
ExpectedErrStr: "duplicate primary key given: [5]",
},
{
Query: "SELECT * FROM test;",
Expected: []sql.Row{{1, 1}, {4, 4}, {5, 5}},
},
},
},
}
Expand Down
24 changes: 24 additions & 0 deletions enginetest/query_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,30 @@ var PlanTests = []QueryPlanTest{
" └─ IndexedTableAccess(two_pk on [two_pk.pk1,two_pk.pk2])\n" +
"",
},
{
Query: `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET two_pk.c1 = two_pk.c1 + 1`,
ExpectedPlan: "Update\n" +
" └─ Update Join\n" +
" └─ UpdateSource(SET two_pk.c1 = (two_pk.c1 + 1))\n" +
" └─ IndexedJoinSorter()\n" +
" └─ IndexedJoin(one_pk.pk = two_pk.pk1)\n" +
" ├─ Table(two_pk)\n" +
" └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
"",
},
{
Query: `UPDATE one_pk INNER JOIN (SELECT * FROM two_pk) as t2 on one_pk.pk = t2.pk1 SET one_pk.c1 = one_pk.c1 + 1, one_pk.c2 = one_pk.c2 + 1`,
ExpectedPlan: "Update\n" +
" └─ Update Join\n" +
" └─ UpdateSource(SET one_pk.c1 = (one_pk.c1 + 1),SET one_pk.c2 = (one_pk.c2 + 1))\n " +
" └─ IndexedJoinSorter()\n" +
" └─ IndexedJoin(one_pk.pk = t2.pk1)\n" +
" ├─ SubqueryAlias(t2)\n" +
" │ └─ Projected table access on [pk1 pk2 c1 c2 c3 c4 c5]\n" +
" │ └─ Table(two_pk)\n" +
" └─ IndexedTableAccess(one_pk on [one_pk.pk])\n" +
"",
},
}

// Queries where the query planner produces a correct (results) but suboptimal plan.
Expand Down
2 changes: 1 addition & 1 deletion enginetest/script_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ var ScriptTests = []ScriptTest{
Expected: []sql.Row{{1, 1}, {4, 4}, {5, 5}},
},
{
Query: "UPDATE test SET pk = pk + 1;",
Query: "UPDATE test SET pk = pk + 1 ORDER BY pk;",
ExpectedErrStr: "duplicate primary key given: [5]",
},
{
Expand Down
151 changes: 135 additions & 16 deletions enginetest/update_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,140 @@ var UpdateTests = []WriteQueryTest{
sql.NewRow(1, 1, 32, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable CROSS JOIN tabletest set othertable.i2 = othertable.i2 * 10`, // cross join
ExpectedWriteResult: []sql.Row{{newUpdateResult(3, 3)}},
SelectQuery: "SELECT * FROM othertable order by i2",
ExpectedSelect: []sql.Row{
sql.NewRow("third", 10),
sql.NewRow("second", 20),
sql.NewRow("first", 30),
},
},
{
WriteQuery: `UPDATE tabletest cross join tabletest as t2 set tabletest.i = tabletest.i * 10`, // cross join
ExpectedWriteResult: []sql.Row{{newUpdateResult(3, 3)}},
SelectQuery: "SELECT * FROM tabletest order by i",
ExpectedSelect: []sql.Row{
sql.NewRow(10, "first row"),
sql.NewRow(20, "second row"),
sql.NewRow(30, "third row"),
},
},
{
WriteQuery: `UPDATE othertable, tabletest set tabletest.i = tabletest.i * 10`, // cross join
ExpectedWriteResult: []sql.Row{{newUpdateResult(3, 3)}},
SelectQuery: "SELECT * FROM tabletest order by i",
ExpectedSelect: []sql.Row{
sql.NewRow(10, "first row"),
sql.NewRow(20, "second row"),
sql.NewRow(30, "third row"),
},
},
{
WriteQuery: `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 INNER JOIN two_pk a1 on one_pk.pk = two_pk.pk2 SET two_pk.c1 = two_pk.c1 + 1`, // cross join
Copy link
Member

Choose a reason for hiding this comment

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

I'm not verifying these query results by hand, make sure you have (or tested them on mysql)

Copy link
Member

Choose a reason for hiding this comment

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

This isn't a cross join (a cross join has no join condition)(

ExpectedWriteResult: []sql.Row{{newUpdateResult(2, 2)}},
SelectQuery: "SELECT * FROM two_pk order by pk1 ASC, pk2 ASC;",
ExpectedSelect: []sql.Row{
sql.NewRow(0, 0, 1, 1, 2, 3, 4),
sql.NewRow(0, 1, 10, 11, 12, 13, 14),
sql.NewRow(1, 0, 20, 21, 22, 23, 24),
sql.NewRow(1, 1, 31, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable INNER JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // cross join
Copy link
Member

Choose a reason for hiding this comment

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

This is a cross join but not phrased as on

A cross join has no join condition

This just reduces to a cross join because the analyzer realizes that none of the join predicates are actually join predicates to moves them to the filter.

You should keep this test and add another one that is an explicit cross join

Copy link
Member

Choose a reason for hiding this comment

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

You should make sure you have tests for the implicit cross join, e.g.

update table1, table2 set...

ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM othertable order by i2",
ExpectedSelect: []sql.Row{
sql.NewRow("third", 1),
sql.NewRow("second", 2),
sql.NewRow("fourth", 3),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // left join
ExpectedWriteResult: []sql.Row{{newUpdateResult(3, 3)}},
SelectQuery: "SELECT * FROM othertable order by i2",
ExpectedSelect: []sql.Row{
sql.NewRow("fourth", 1),
sql.NewRow("fourth", 2),
sql.NewRow("fourth", 3),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET tabletest.s = 'fourth row', tabletest.i = tabletest.i + 1`, // left join
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM tabletest order by i",
ExpectedSelect: []sql.Row{
sql.NewRow(1, "first row"),
sql.NewRow(2, "second row"),
sql.NewRow(4, "fourth row"),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 LEFT JOIN one_pk on othertable.i2 = one_pk.pk SET one_pk.c1 = one_pk.c1 + 1`, // left join
ExpectedWriteResult: []sql.Row{{newUpdateResult(3, 3)}},
SelectQuery: "SELECT * FROM one_pk order by pk",
ExpectedSelect: []sql.Row{
sql.NewRow(0, 0, 1, 2, 3, 4),
sql.NewRow(1, 11, 11, 12, 13, 14),
sql.NewRow(2, 21, 21, 22, 23, 24),
sql.NewRow(3, 31, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 LEFT JOIN one_pk on othertable.i2 = one_pk.pk SET one_pk.c1 = one_pk.c1 + 1 where one_pk.pk > 4`, // left join
ExpectedWriteResult: []sql.Row{{newUpdateResult(0, 0)}},
SelectQuery: "SELECT * FROM one_pk order by pk",
ExpectedSelect: []sql.Row{
sql.NewRow(0, 0, 1, 2, 3, 4),
sql.NewRow(1, 10, 11, 12, 13, 14),
sql.NewRow(2, 20, 21, 22, 23, 24),
sql.NewRow(3, 30, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 LEFT JOIN one_pk on othertable.i2 = 1 and one_pk.pk = 1 SET one_pk.c1 = one_pk.c1 + 1`, // left join
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM one_pk order by pk",
ExpectedSelect: []sql.Row{
sql.NewRow(0, 0, 1, 2, 3, 4),
sql.NewRow(1, 11, 11, 12, 13, 14),
sql.NewRow(2, 20, 21, 22, 23, 24),
sql.NewRow(3, 30, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable RIGHT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // right join
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM othertable order by i2",
ExpectedSelect: []sql.Row{
sql.NewRow("third", 1),
sql.NewRow("second", 2),
sql.NewRow("fourth", 3),
},
},
{
WriteQuery: `UPDATE othertable RIGHT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.i2 = othertable.i2 + 1`, // right join
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM othertable order by i2",
ExpectedSelect: []sql.Row{
sql.NewRow("third", 1),
sql.NewRow("second", 2),
sql.NewRow("first", 4),
},
},
{
WriteQuery: `UPDATE othertable LEFT JOIN tabletest on othertable.i2=tabletest.i RIGHT JOIN one_pk on othertable.i2 = 1 and one_pk.pk = 1 SET tabletest.s = 'updated';`, // right join
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
SelectQuery: "SELECT * FROM tabletest order by i",
ExpectedSelect: []sql.Row{
sql.NewRow(1, "updated"),
sql.NewRow(2, "second row"),
sql.NewRow(3, "third row"),
},
},
}

// These tests return the correct select query answer but the wrong write result.
Expand All @@ -276,17 +410,6 @@ var SkippedUpdateTests = []WriteQueryTest{
sql.NewRow(1, 1, 32, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 INNER JOIN two_pk a1 on one_pk.pk = two_pk.pk2 SET two_pk.c1 = two_pk.c1 + 1`,
ExpectedWriteResult: []sql.Row{{newUpdateResult(2, 2)}},
SelectQuery: "SELECT * FROM two_pk;",
ExpectedSelect: []sql.Row{
sql.NewRow(0, 0, 1, 1, 2, 3, 4),
sql.NewRow(0, 1, 10, 11, 12, 13, 14),
sql.NewRow(1, 0, 20, 21, 22, 23, 24),
sql.NewRow(1, 1, 31, 31, 32, 33, 34),
},
},
{
WriteQuery: `UPDATE othertable INNER JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`,
ExpectedWriteResult: []sql.Row{{newUpdateResult(1, 1)}},
Expand Down Expand Up @@ -371,11 +494,7 @@ var GenericUpdateErrorTests = []GenericErrorQueryTest{

var UpdateErrorTests = []QueryErrorTest{
{
Query: `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 INNER JOIN two_pk a1 on one_pk.pk = two_pk.pk2 SET two_pk.c1 = two_pk.c1 + 1`,
ExpectedErr: sql.ErrUnsupportedFeature,
},
{
Query: `UPDATE othertable INNER JOIN tabletest on othertable.i2=3 and tabletest.s=3 SET othertable.s2 = 'fourth'`,
Query: `UPDATE keyless INNER JOIN one_pk on keyless.c0 = one_pk.pk SET keyless.c0 = keyless.c0 + 1`,
ExpectedErr: sql.ErrUnsupportedFeature,
},
}
2 changes: 1 addition & 1 deletion memory/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func NewViewlessDatabase(name string) *BaseDatabase {
}
}

// EnablePrimaryKeyIndexes causes every table created in this database to use an index on its primary keys
// EnablePrimaryKeyIndexes causes every table created in this database to use an index on its primary partitionKeys
func (d *BaseDatabase) EnablePrimaryKeyIndexes() {
d.primaryKeyIndexes = true
}
Expand Down
Loading