From 05d4e649a55e4a4e1e0271e9ef47e56e6b5f9d87 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Nov 2024 14:48:13 -0800 Subject: [PATCH 1/7] Updated skip list TestUpdate --- testing/go/enginetest/doltgres_engine_test.go | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go index 4a48ddf6f..95a8cb3b0 100755 --- a/testing/go/enginetest/doltgres_engine_test.go +++ b/testing/go/enginetest/doltgres_engine_test.go @@ -422,8 +422,39 @@ func TestReplaceIntoErrors(t *testing.T) { } func TestUpdate(t *testing.T) { - t.Skip() - h := newDoltgresServerHarness(t) + h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ + "UPDATE mytable SET s = _binary 'updated' WHERE i = 3;", // _binary not supported + "UPDATE mytable SET s = 'updated' ORDER BY i LIMIT 1 OFFSET 1;", // offset not supported (limit isn't selected in vanilla postgres but is in the cockroach grammar) + // TODO: Postgres supports update joins but with a very different syntax, and some join types are not supported + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET two_pk.c1 = two_pk.c1 + 1`, + "UPDATE mytable INNER JOIN one_pk ON mytable.i = one_pk.c5 SET mytable.i = mytable.i * 10", + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET two_pk.c1 = two_pk.c1 + 1 WHERE one_pk.c5 < 10`, + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 INNER JOIN othertable on othertable.i2 = two_pk.pk2 SET one_pk.c1 = one_pk.c1 + 1`, + `UPDATE one_pk INNER JOIN (SELECT * FROM two_pk order by pk1, pk2) as t2 on one_pk.pk = t2.pk1 SET one_pk.c1 = t2.c1 + 1 where one_pk.pk < 1`, + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET one_pk.c1 = one_pk.c1 + 1`, + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET one_pk.c1 = one_pk.c1 + 1, one_pk.c2 = one_pk.c2 + 1 ORDER BY one_pk.pk`, + `UPDATE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET one_pk.c1 = one_pk.c1 + 1, two_pk.c1 = two_pk.c2 + 1`, + `update mytable h join mytable on h.i = mytable.i and h.s <> mytable.s set h.i = mytable.i+1;`, + `UPDATE othertable CROSS JOIN tabletest set othertable.i2 = othertable.i2 * 10`, // cross join + `UPDATE tabletest cross join tabletest as t2 set tabletest.i = tabletest.i * 10`, // cross join + `UPDATE othertable cross join tabletest set tabletest.i = tabletest.i * 10`, // cross join + `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 + `UPDATE othertable INNER JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // cross join + `UPDATE tabletest cross join tabletest as t2 set t2.i = t2.i * 10`, // cross join + `UPDATE othertable LEFT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // left join + `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 + `UPDATE othertable LEFT JOIN tabletest t3 on othertable.i2=3 and t3.i=3 SET t3.s = 'fourth row', t3.i = t3.i + 1`, // left join + `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 + `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 + `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 + `UPDATE othertable RIGHT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.s2 = 'fourth'`, // right join + `UPDATE othertable RIGHT JOIN tabletest on othertable.i2=3 and tabletest.i=3 SET othertable.i2 = othertable.i2 + 1`, // right join + `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 + `UPDATE IGNORE one_pk INNER JOIN two_pk on one_pk.pk = two_pk.pk1 SET two_pk.c1 = two_pk.c1 + 1`, + `UPDATE IGNORE one_pk JOIN one_pk one_pk2 on one_pk.pk = one_pk2.pk SET one_pk.pk = 10`, + `with t (n) as (select (1) from dual) UPDATE mytable set s = concat('updated ', i) where i in (select n from t)`, // with not supported + `with recursive t (n) as (select (1) from dual union all select n + 1 from t where n < 2) UPDATE mytable set s = concat('updated ', i) where i in (select n from t)`, + }) defer h.Close() enginetest.TestUpdate(t, h) } From c54fdc31a80aa70a0dd80b1621a3b7f659bcff65 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 13 Nov 2024 13:49:02 -0800 Subject: [PATCH 2/7] Bug fix for date column keys and update tests --- server/types/date.go | 2 +- testing/go/enginetest/doltgres_engine_test.go | 56 ++++++++----------- .../go/enginetest/doltgres_harness_test.go | 8 ++- 3 files changed, 29 insertions(+), 37 deletions(-) diff --git a/server/types/date.go b/server/types/date.go index 2d26efb29..2e11b62f7 100644 --- a/server/types/date.go +++ b/server/types/date.go @@ -209,7 +209,7 @@ func (b DateType) ToArrayType() DoltgresArrayType { // Type implements the DoltgresType interface. func (b DateType) Type() query.Type { - return sqltypes.Text + return sqltypes.Date } // ValueType implements the DoltgresType interface. diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go index 95a8cb3b0..ee8660dd2 100755 --- a/testing/go/enginetest/doltgres_engine_test.go +++ b/testing/go/enginetest/doltgres_engine_test.go @@ -171,35 +171,23 @@ func TestSingleScript(t *testing.T) { var scripts = []queries.ScriptTest{ { - Name: "Insert throws primary key violations", + Name: "people tables", SetUpScript: []string{ - "CREATE TABLE t (pk int PRIMARY key);", - "CREATE TABLE t2 (pk1 int, pk2 int, PRIMARY KEY (pk1, pk2));", + "CREATE TABLE `people` ( `dob` date NOT NULL," + + " `first_name` varchar(20) NOT NULL," + + " `last_name` varchar(20) NOT NULL," + + " `middle_name` varchar(20) NOT NULL," + + " `height_inches` bigint NOT NULL," + + " `gender` bigint NOT NULL," + + " PRIMARY KEY (`dob`,`first_name`,`last_name`,`middle_name`) )", + `insert into people values ('1970-12-1'::date, 'jon', 'smith', 'a', 72, 0)`, }, Assertions: []queries.ScriptTestAssertion{ { - Query: "INSERT INTO t VALUES (1), (2);", - Expected: []sql.Row{{types.NewOkResult(2)}}, - }, - { - Query: "INSERT into t VALUES (1);", - ExpectedErr: sql.ErrPrimaryKeyViolation, - }, - { - Query: "SELECT * from t;", - Expected: []sql.Row{{1}, {2}}, - }, - { - Query: "INSERT into t2 VALUES (1, 1), (2, 2);", - Expected: []sql.Row{{types.NewOkResult(2)}}, - }, - { - Query: "INSERT into t2 VALUES (1, 1);", - ExpectedErr: sql.ErrPrimaryKeyViolation, - }, - { - Query: "SELECT * from t2;", - Expected: []sql.Row{{1, 1}, {2, 2}}, + Query: "select * from people order by dob", + Expected: []sql.Row{ + {"1970-12-01", "jon", "smith", "a", int64(72), int64(0)}, + }, }, }, }, @@ -459,16 +447,16 @@ func TestUpdate(t *testing.T) { enginetest.TestUpdate(t, h) } -func TestUpdateIgnore(t *testing.T) { - t.Skip() - h := newDoltgresServerHarness(t) - defer h.Close() - enginetest.TestUpdateIgnore(t, h) -} - func TestUpdateErrors(t *testing.T) { - t.Skip() - h := newDoltgresServerHarness(t) + h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ + `UPDATE keyless INNER JOIN one_pk on keyless.c0 = one_pk.pk SET keyless.c0 = keyless.c0 + 1`, + // `UPDATE people set height_inches = null where height_inches < 100`, + // `UPDATE people SET height_inches = IF(SUM(height_inches) % 2 = 0, 42, height_inches)`, + // `UPDATE people SET height_inches = IF(SUM(*) % 2 = 0, 42, height_inches)`, + // `UPDATE people SET height_inches = IF(ROW_NUMBER() OVER() % 2 = 0, 42, height_inches)`, + "try updating string that is too long", // works but error message doesn't match + "UPDATE mytable SET s = 'hi' LIMIT -1;", // unsupported syntax + }) defer h.Close() enginetest.TestUpdateErrors(t, h) } diff --git a/testing/go/enginetest/doltgres_harness_test.go b/testing/go/enginetest/doltgres_harness_test.go index fc0610f7a..e74c07b6f 100644 --- a/testing/go/enginetest/doltgres_harness_test.go +++ b/testing/go/enginetest/doltgres_harness_test.go @@ -543,8 +543,12 @@ func (d *DoltgresHarness) EvaluateExpectedError(t *testing.T, expected string, e // EvaluateExpectedErrorKind is a harness extension that gives us more control over matching expected errors. We don't // have access to the error kind object eny longer, so we have to see if the error we get matches its pattern func (d *DoltgresHarness) EvaluateExpectedErrorKind(t *testing.T, expected *errors.Kind, actualErr error) { - pattern := strings.ReplaceAll(expected.Message, "%s", "\\w+") - pattern = strings.ReplaceAll(pattern, "%q", "\"\\w+\"") + pattern := strings.ReplaceAll(expected.Message, "*", "\\*") + pattern = strings.ReplaceAll(pattern, "(", "\\(") + pattern = strings.ReplaceAll(pattern, ")", "\\)") + pattern = strings.ReplaceAll(pattern, "%s", ".+") + pattern = strings.ReplaceAll(pattern, "%q", "\".+\"") + pattern = strings.ReplaceAll(pattern, "%v", ".+?") regex, regexErr := regexp.Compile(pattern) require.NoError(t, regexErr) From 955f876565d79e6cdf57494cf55aa2e040e469b0 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 13 Nov 2024 15:25:35 -0800 Subject: [PATCH 3/7] Lots of tests for using various types as keys --- testing/go/types_test.go | 283 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 283 insertions(+) diff --git a/testing/go/types_test.go b/testing/go/types_test.go index a562a0e9d..314b32cf0 100644 --- a/testing/go/types_test.go +++ b/testing/go/types_test.go @@ -41,6 +41,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Bigint key", + SetUpScript: []string{ + "CREATE TABLE t_bigint (id BIGINT primary key, v1 BIGINT);", + "INSERT INTO t_bigint VALUES (1, 123456789012345), (2, 987654321098765);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_bigint WHERE id = 1 ORDER BY id;", + Expected: []sql.Row{ + {1, 123456789012345}, + }, + }, + }, + }, { Name: "Bigint array type", SetUpScript: []string{ @@ -75,6 +90,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Bit key", + Skip: true, + SetUpScript: []string{ + "CREATE TABLE t_bit (id BIT(8) primary key, v1 BIT(8));", + "INSERT INTO t_bit VALUES (B'11011010', B'11011010'), (B'00101011', B'00101011');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_bit WHERE id = B'11011010' ORDER BY id;", + Expected: []sql.Row{ + {[]byte{0xDA}, []byte{0xDA}}, + }, + }, + }, + }, { Name: "Boolean type", SetUpScript: []string{ @@ -116,6 +147,23 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Boolean key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_boolean (id boolean primary key, v1 BOOLEAN);", + "INSERT INTO t_boolean VALUES (true, true), (false, 'false')", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_boolean where id ORDER BY id;", + Skip: true, // Proper NULL-ordering has not yet been implemented + Expected: []sql.Row{ + {"t", "t"}, + }, + }, + }, + }, { Name: "Boolean array type", SetUpScript: []string{ @@ -189,6 +237,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Bigserial key", + SetUpScript: []string{ + "CREATE TABLE t_bigserial (id BIGSERIAL primary key, v1 BIGSERIAL);", + "INSERT INTO t_bigserial VALUES (123456789012345, 123456789012345), (987654321098765, 987654321098765);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_bigserial where ID = 987654321098765 ORDER BY id;", + Expected: []sql.Row{ + {987654321098765, 987654321098765}, + }, + }, + }, + }, { Name: "Bit varying type", Skip: true, @@ -242,6 +305,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Bytea key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_bytea (id BYTEA primary key, v1 BYTEA);", + "INSERT INTO t_bytea VALUES (E'\\\\xCAFEBABE', E'\\\\xDEADBEEF'), ('\\xBADD00D5', '\\xC0FFEE');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_bytea WHERE ID = E'\\\\xCAFEBABE' ORDER BY id;", + Expected: []sql.Row{ + {[]byte{0xCA, 0xFE, 0xBA, 0xBE}, []byte{0xDE, 0xAD, 0xBE, 0xEF}}, + }, + }, + }, + }, { Name: "Character type", SetUpScript: []string{ @@ -279,6 +358,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Character key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_character (id CHAR(5) primary key, v1 CHARACTER(5));", + "INSERT INTO t_character VALUES ('abcde', 'fghjk'), ('vwxyz', '12345')", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_character WHERE ID = 'vwxyz' ORDER BY id;", + Expected: []sql.Row{ + {"vwxyz", "12345"}, + }, + }, + }, + }, { Name: "Internal char type", SetUpScript: []string{ @@ -605,6 +700,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Date key", + SetUpScript: []string{ + "CREATE TABLE t_date (id DATE primary key, v1 DATE);", + "INSERT INTO t_date VALUES ('2025-01-01', '2023-01-01'), ('2026-01-01', '2023-02-02');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_date where Id = '2025-01-01' ORDER BY id;", + Expected: []sql.Row{ + {"2025-01-01", "2023-01-01"}, + }, + }, + }, + }, { Name: "Double precision type", SetUpScript: []string{ @@ -621,6 +731,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Double precision key", + SetUpScript: []string{ + "CREATE TABLE t_double_precision (id DOUBLE PRECISION primary key, v1 DOUBLE PRECISION);", + "INSERT INTO t_double_precision VALUES (456.789, 123.456), (123.456, 789.012);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_double_precision WHERE id = 456.789 ORDER BY id;", + Expected: []sql.Row{ + {456.789, 123.456}, + }, + }, + }, + }, { Name: "Double precision array type", SetUpScript: []string{ @@ -764,6 +889,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Interval key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_interval (id interval primary key, v1 INTERVAL);", + "INSERT INTO t_interval VALUES ('1 hour', '1 day 3 hours'), ('2 days', '23 hours 30 minutes');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_interval WHERE id = '1 hour' ORDER BY id;", + Expected: []sql.Row{ + {"01:00:00", "1 day 03:00:00"}, + }, + }, + }, + }, { Name: "Interval array type", SetUpScript: []string{ @@ -780,6 +921,17 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "JSON key", + SetUpScript: []string{}, + Assertions: []ScriptTestAssertion{ + { + Query: "CREATE TABLE t_json (id JSON primary key, v1 JSON);", + ExpectedErr: "data type json has no default operator class for access method \"btree\"", + Skip: true, // current error message is blob/text column 'id' used in key specification without a key length + }, + }, + }, { Name: "JSON type", SetUpScript: []string{ @@ -1235,6 +1387,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Name key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_name (id NAME primary key, v1 NAME);", + "INSERT INTO t_name VALUES ('wxyz', 'abcdefghij'), ('abcd', 'klmnopqrst');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_name WHERE id = 'wxyz' ORDER BY id;", + Expected: []sql.Row{ + {"wxyz", "abcdefghij"}, + }, + }, + }, + }, { Name: "Name type, explicit casts", SetUpScript: []string{ @@ -1398,6 +1566,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Numeric key", + Skip: true, // error decoding binary: Int.GobDecode: encoding version 57 not supported + SetUpScript: []string{ + "CREATE TABLE t_numeric (id numeric(5,2) primary key, v1 NUMERIC(5,2));", + "INSERT INTO t_numeric VALUES (123.45, 67.89), (67.89, 100.3);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_numeric WHERE ID = 123.45 ORDER BY id;", + Expected: []sql.Row{ + {Numeric("123.45"), Numeric("67.89")}, + }, + }, + }, + }, { Name: "Numeric type, no scale or precision", SetUpScript: []string{ @@ -1776,6 +1960,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Real key", + SetUpScript: []string{ + "CREATE TABLE t_real (id REAL primary key, v1 REAL);", + "INSERT INTO t_real VALUES (123.875, 67.125), (67.125, 123.875);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_real WHERE ID = 123.875 ORDER BY id;", + Expected: []sql.Row{ + {123.875, 67.125}, + }, + }, + }, + }, { Name: "Real array type", SetUpScript: []string{ @@ -2090,6 +2289,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Smallint key", + SetUpScript: []string{ + "CREATE TABLE t_smallint (id smallint primary key, v1 SMALLINT);", + "INSERT INTO t_smallint VALUES (1, 42), (2, 99);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_smallint WHERE ID = 1 ORDER BY id;", + Expected: []sql.Row{ + {1, 42}, + }, + }, + }, + }, { Name: "Smallint array type", SetUpScript: []string{ @@ -2122,6 +2336,21 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Smallserial key", + SetUpScript: []string{ + "CREATE TABLE t_smallserial (id smallserial primary key, v1 SMALLSERIAL);", + "INSERT INTO t_smallserial (v1) VALUES (42), (99);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_smallserial WHERE ID = 1 ORDER BY id;", + Expected: []sql.Row{ + {1, 42}, + }, + }, + }, + }, { Name: "Serial type", SetUpScript: []string{ @@ -2136,6 +2365,12 @@ var typesTests = []ScriptTest{ {2, 456}, }, }, + { + Query: "SELECT * FROM t_serial WHERE ID = 2 ORDER BY id;", + Expected: []sql.Row{ + {2, 456}, + }, + }, }, }, { @@ -2251,6 +2486,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Text key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_text (id TEXT primary key, v1 TEXT);", + "INSERT INTO t_text VALUES ('Hello', 'World'), ('goodbye', 'cruel world');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_text where id = 'goodbye' ORDER BY id;", + Expected: []sql.Row{ + {"goodbye", "cruel world"}, + }, + }, + }, + }, { Name: "Time without time zone type", SetUpScript: []string{ @@ -2290,6 +2541,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Time without time zone key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_time_without_zone (id TIME primary key, v1 TIME);", + "INSERT INTO t_time_without_zone VALUES ('12:34:56', '23:45:01'), ('23:45:01', '12:34:56');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_time_without_zone WHERE ID = '12:34:56' ORDER BY id;", + Expected: []sql.Row{ + {"12:34:56", "23:45:01"}, + }, + }, + }, + }, { // TODO: timezone representation is reported via local time, need to account for that in testing? Name: "Time with time zone type", SetUpScript: []string{ @@ -2458,6 +2725,22 @@ var typesTests = []ScriptTest{ }, }, }, + { + Name: "Uuid key", + Skip: true, // blob/text column 'id' used in key specification without a key length + SetUpScript: []string{ + "CREATE TABLE t_uuid (id UUID primary key, v1 UUID);", + "INSERT INTO t_uuid VALUES ('f47ac10b58cc4372a567-0e02b2c3d479', 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11'), ('a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', 'f47ac10b58cc4372a567-0e02b2c3d479');", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "SELECT * FROM t_uuid WHERE ID = 'f47ac10b58cc4372a567-0e02b2c3d479' ORDER BY id;", + Expected: []sql.Row{ + {"f47ac10b58cc4372a567-0e02b2c3d479", "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11"}, + }, + }, + }, + }, { Name: "Uuid array type", SetUpScript: []string{ From 181ed90dfd6b8cfe1cd26f714dfb522517a547ea Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 13 Nov 2024 17:14:58 -0800 Subject: [PATCH 4/7] Delete tests --- testing/go/enginetest/doltgres_engine_test.go | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go index ee8660dd2..631dee7a5 100755 --- a/testing/go/enginetest/doltgres_engine_test.go +++ b/testing/go/enginetest/doltgres_engine_test.go @@ -450,10 +450,6 @@ func TestUpdate(t *testing.T) { func TestUpdateErrors(t *testing.T) { h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ `UPDATE keyless INNER JOIN one_pk on keyless.c0 = one_pk.pk SET keyless.c0 = keyless.c0 + 1`, - // `UPDATE people set height_inches = null where height_inches < 100`, - // `UPDATE people SET height_inches = IF(SUM(height_inches) % 2 = 0, 42, height_inches)`, - // `UPDATE people SET height_inches = IF(SUM(*) % 2 = 0, 42, height_inches)`, - // `UPDATE people SET height_inches = IF(ROW_NUMBER() OVER() % 2 = 0, 42, height_inches)`, "try updating string that is too long", // works but error message doesn't match "UPDATE mytable SET s = 'hi' LIMIT -1;", // unsupported syntax }) @@ -462,10 +458,22 @@ func TestUpdateErrors(t *testing.T) { } func TestDeleteFrom(t *testing.T) { - t.Skip() - h := newDoltgresServerHarness(t) + h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ + "DELETE FROM mytable ORDER BY i DESC LIMIT 1 OFFSET 1;", // offset is unsupported syntax + "DELETE FROM mytable WHERE (i,s) = (1, 'first row');", // type error, needs investigation + "with t (n) as (select (1) from dual) delete from mytable where i in (select n from t)", + "with recursive t (n) as (select (1) from dual union all select n + 1 from t where n < 2) delete from mytable where i in (select n from t)", + }) defer h.Close() - enginetest.TestDelete(t, h) + + // We've inlined part of engineTest.TestDeleteFrom here because that method tests many queries for join deletions + // that would be tedious to write out as skips + h.Setup(setup.MydbData, setup.MytableData, setup.TabletestData) + t.Run("Delete from single table", func(t *testing.T) { + for _, tt := range queries.DeleteTests { + enginetest.RunWriteQueryTest(t, h, tt) + } + }) } func TestDeleteFromErrors(t *testing.T) { From 4e4050f51e11bf90297fe495282b25a7a941f643 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 13 Nov 2024 17:53:21 -0800 Subject: [PATCH 5/7] Delete from error tests, and added bounds checking to limit and offset --- server/ast/limit.go | 42 +++++++++++++++++++ testing/go/enginetest/doltgres_engine_test.go | 36 +++++++++++++++- 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/server/ast/limit.go b/server/ast/limit.go index 69de6a540..e5337b1b1 100644 --- a/server/ast/limit.go +++ b/server/ast/limit.go @@ -15,6 +15,8 @@ package ast import ( + "fmt" + vitess "github.com/dolthub/vitess/go/vt/sqlparser" pgexprs "github.com/dolthub/doltgresql/server/expression" @@ -43,11 +45,31 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { // We need to remove the hard dependency, but for now, we'll just convert our literals to a vitess.SQLVal. if injectedExpr, ok := count.(vitess.InjectedExpr); ok { if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok { + l := literal.Value() + limitValue, err := int64Value(l) + if err != nil { + return nil, err + } + + if limitValue < 0 { + return nil, fmt.Errorf("LIMIT must be greater than or equal to 0") + } + count = literal.ToVitessLiteral() } } if injectedExpr, ok := offset.(vitess.InjectedExpr); ok { if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok { + o := literal.Value() + offsetVal, err := int64Value(o) + if err != nil { + return nil, err + } + + if offsetVal < 0 { + return nil, fmt.Errorf("OFFSET must be greater than or equal to 0") + } + offset = literal.ToVitessLiteral() } } @@ -56,3 +78,23 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { Rowcount: count, }, nil } + +// int64Value converts a literal value to an int64 +func int64Value(l any) (int64, error) { + var limitValue int64 + switch l.(type) { + case int: + limitValue = int64(l.(int)) + case int32: + limitValue = int64(l.(int32)) + case int64: + limitValue = l.(int64) + case float64: + limitValue = int64(l.(float64)) + case float32: + limitValue = int64(l.(float32)) + default: + return 0, fmt.Errorf("unsupported limit value type %T", l) + } + return limitValue, nil +} diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go index 631dee7a5..6215843d2 100755 --- a/testing/go/enginetest/doltgres_engine_test.go +++ b/testing/go/enginetest/doltgres_engine_test.go @@ -477,10 +477,42 @@ func TestDeleteFrom(t *testing.T) { } func TestDeleteFromErrors(t *testing.T) { - t.Skip() h := newDoltgresServerHarness(t) defer h.Close() - enginetest.TestDeleteErrors(t, h) + + // These tests are overspecified to mysql-specific errors and include some syntax we don't support, so we redefine + // the subset we're interested in checking here + h.Setup(setup.MydbData, setup.MytableData, setup.TabletestData) + deleteScripts := []queries.ScriptTest{ + { + Name: "DELETE FROM error cases", + Assertions: []queries.ScriptTestAssertion{ + { + Query: "DELETE FROM invalidtable WHERE x < 1;", + ExpectedErrStr: "table not found: invalidtable", + }, + { + Query: "DELETE FROM mytable WHERE z = 'dne';", + ExpectedErrStr: "column \"z\" could not be found in any table in scope", + }, + { + Query: "DELETE FROM mytable LIMIT -1;", + ExpectedErrStr: "LIMIT must be greater than or equal to 0", + }, + { + Query: "DELETE mytable WHERE i = 1;", + ExpectedErrStr: "syntax error", + }, + { + Query: "DELETE FROM (SELECT * FROM mytable) mytable WHERE i = 1;", + ExpectedErrStr: "syntax error", + }, + }, + }, + } + for _, tt := range deleteScripts { + enginetest.TestScript(t, h, tt) + } } func TestSpatialDelete(t *testing.T) { From 57a614663d6505c7cb02507c0374662a33d3e509 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 14 Nov 2024 12:12:11 -0800 Subject: [PATCH 6/7] Format and linting --- server/ast/limit.go | 15 +++++++-------- testing/go/enginetest/doltgres_engine_test.go | 8 ++++---- testing/go/types_test.go | 6 +++--- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/server/ast/limit.go b/server/ast/limit.go index e5337b1b1..e7c9f7e74 100644 --- a/server/ast/limit.go +++ b/server/ast/limit.go @@ -19,9 +19,8 @@ import ( vitess "github.com/dolthub/vitess/go/vt/sqlparser" - pgexprs "github.com/dolthub/doltgresql/server/expression" - "github.com/dolthub/doltgresql/postgres/parser/sem/tree" + pgexprs "github.com/dolthub/doltgresql/server/expression" ) // nodeLimit handles *tree.Limit nodes. @@ -82,17 +81,17 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { // int64Value converts a literal value to an int64 func int64Value(l any) (int64, error) { var limitValue int64 - switch l.(type) { + switch l := l.(type) { case int: - limitValue = int64(l.(int)) + limitValue = int64(l) case int32: - limitValue = int64(l.(int32)) + limitValue = int64(l) case int64: - limitValue = l.(int64) + limitValue = l case float64: - limitValue = int64(l.(float64)) + limitValue = int64(l) case float32: - limitValue = int64(l.(float32)) + limitValue = int64(l) default: return 0, fmt.Errorf("unsupported limit value type %T", l) } diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go index 6215843d2..65d7f2c71 100755 --- a/testing/go/enginetest/doltgres_engine_test.go +++ b/testing/go/enginetest/doltgres_engine_test.go @@ -450,8 +450,8 @@ func TestUpdate(t *testing.T) { func TestUpdateErrors(t *testing.T) { h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ `UPDATE keyless INNER JOIN one_pk on keyless.c0 = one_pk.pk SET keyless.c0 = keyless.c0 + 1`, - "try updating string that is too long", // works but error message doesn't match - "UPDATE mytable SET s = 'hi' LIMIT -1;", // unsupported syntax + "try updating string that is too long", // works but error message doesn't match + "UPDATE mytable SET s = 'hi' LIMIT -1;", // unsupported syntax }) defer h.Close() enginetest.TestUpdateErrors(t, h) @@ -460,7 +460,7 @@ func TestUpdateErrors(t *testing.T) { func TestDeleteFrom(t *testing.T) { h := newDoltgresServerHarness(t).WithSkippedQueries([]string{ "DELETE FROM mytable ORDER BY i DESC LIMIT 1 OFFSET 1;", // offset is unsupported syntax - "DELETE FROM mytable WHERE (i,s) = (1, 'first row');", // type error, needs investigation + "DELETE FROM mytable WHERE (i,s) = (1, 'first row');", // type error, needs investigation "with t (n) as (select (1) from dual) delete from mytable where i in (select n from t)", "with recursive t (n) as (select (1) from dual union all select n + 1 from t where n < 2) delete from mytable where i in (select n from t)", }) @@ -479,7 +479,7 @@ func TestDeleteFrom(t *testing.T) { func TestDeleteFromErrors(t *testing.T) { h := newDoltgresServerHarness(t) defer h.Close() - + // These tests are overspecified to mysql-specific errors and include some syntax we don't support, so we redefine // the subset we're interested in checking here h.Setup(setup.MydbData, setup.MytableData, setup.TabletestData) diff --git a/testing/go/types_test.go b/testing/go/types_test.go index 314b32cf0..fce448bb3 100644 --- a/testing/go/types_test.go +++ b/testing/go/types_test.go @@ -922,13 +922,13 @@ var typesTests = []ScriptTest{ }, }, { - Name: "JSON key", + Name: "JSON key", SetUpScript: []string{}, Assertions: []ScriptTestAssertion{ { - Query: "CREATE TABLE t_json (id JSON primary key, v1 JSON);", + Query: "CREATE TABLE t_json (id JSON primary key, v1 JSON);", ExpectedErr: "data type json has no default operator class for access method \"btree\"", - Skip: true, // current error message is blob/text column 'id' used in key specification without a key length + Skip: true, // current error message is blob/text column 'id' used in key specification without a key length }, }, }, From 3cb4f5a172be0d45e9720d6bae4850c39c00af95 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 14 Nov 2024 15:09:29 -0800 Subject: [PATCH 7/7] Rename func --- server/ast/limit.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/ast/limit.go b/server/ast/limit.go index e7c9f7e74..62295a4ee 100644 --- a/server/ast/limit.go +++ b/server/ast/limit.go @@ -45,7 +45,7 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { if injectedExpr, ok := count.(vitess.InjectedExpr); ok { if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok { l := literal.Value() - limitValue, err := int64Value(l) + limitValue, err := int64ValueForLimit(l) if err != nil { return nil, err } @@ -60,7 +60,7 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { if injectedExpr, ok := offset.(vitess.InjectedExpr); ok { if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok { o := literal.Value() - offsetVal, err := int64Value(o) + offsetVal, err := int64ValueForLimit(o) if err != nil { return nil, err } @@ -78,8 +78,8 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) { }, nil } -// int64Value converts a literal value to an int64 -func int64Value(l any) (int64, error) { +// int64ValueForLimit converts a literal value to an int64 +func int64ValueForLimit(l any) (int64, error) { var limitValue int64 switch l := l.(type) { case int: @@ -93,7 +93,7 @@ func int64Value(l any) (int64, error) { case float32: limitValue = int64(l) default: - return 0, fmt.Errorf("unsupported limit value type %T", l) + return 0, fmt.Errorf("unsupported limit/offset value type %T", l) } return limitValue, nil }