7 Jul
2019
7 Jul
'19
9:46 a.m.
Hi Rucha!
Here is the promised review. I have run the main test suite locally and
have noticedthat some tests fail if you run with a debug build. The
failure is an assertionfailure on the protocol level. This means you
forgot to call a my_ok or my_errorat an appropriate time. I suggest you
try and find which one of your commitsintroduces this bug. A handy tool
is `git bisect`. It's a great time to learnabout it, if you have not
used it before.
There are quite a few other things to fix, but first, let's tackle
theinitial review comments.
Now, on to the actual code.
Your text editor inserts tabs instead of spaces. Please configure it to
onlyinsert tabs and have the "tab" size be equal to 2 spaces.
> diff --git a/mysql-test/main/insert_parser.test b/mysql-
test/main/insert_parser.test> new file mode 100644> index
00000000000..5c1f9408a96> --- /dev/null> +++ b/mysql-
test/main/insert_parser.test> @@ -0,0 +1,64 @@> +#> +# Syntax check for
INSERT...RETURNING> +#> +> +--echo # RETURNING * and RETURNING
<COLUMN_NAME> should work. Trailing whitespace here, please fix.> +
--echo # Other cases with RETURNING should output syntax error.> +> +
--disable_warnings> +drop table if exists t1,t2;> +
--enable_warningsTest cases in MariaDB are written such that they do
not leave behind anytables or other elements from running the test.
Thus, such statements arenot required (and actively discouraged) in new
test cases. If tables fromprevious tests are present and mysql-test-run
(mtr) does not mark the test asfailed then inserting such statements
will hide the mysql-test-run bug.Please delete.> +> +CREATE TABLE
t1(id1 INT, val1 VARCHAR(1));> +--echo Table t1 created successfully.
Fields: id1 INT, val1 VARCHAR(1);This comment is not necessary, please
delete. The fact that mtr passed meansthe command was succesful. Also,
when adding comments, I recommend you prefixthem with #, like you did
for the start of the test.> +> +CREATE TABLE t2(id2 INT, val2
VARCHAR(1));> +--echo Table t2 created successfully. Fields: id2 INT,
val2 VARCHAR(1);Same as previous comment, not necessary, please
delete.> +> +#> +#Simple insert statement> +#> +--echo #Simple insert
statementOnly keep one, I suggest the --echo variant, but put a space
after #.I also like to add 2 extra --echo # like so:--echo #--echo #
Simple insert statement--echo #
This makes the result file easier to read.> +INSERT INTO t1 (id1, val1)
VALUES (1, 'a');> +INSERT INTO t1 (id1, val1) VALUES (2, 'b') RETURNING
*;> +INSERT INTO t1 (id1, val1) VALUES (3, 'c') RETURNING id1, val1;>
+INSERT INTO t1 (id1, val1) VALUES (3, 'c') RETURNING id1 AS id;>
+SELECT * FROM t1;> +TRUNCATE TABLE t1;> +SELECT * FROM t1;> +> +#>
+#multiple values in one insert statement> +#> +--echo #multiple values
in one insert statementSame as above.> +INSERT INTO t1 VALUES
(1,'a'),(2,'b');> +INSERT INTO t1 VALUES (3,'c'),(4,'d') RETURNING *;>
+INSERT INTO t1 VALUES (5,'e'),(6,'f') RETURNING id1, val1;> +INSERT
INTO t1 VALUES (5,'e'),(6,'f') RETURNING id1 AS id;> +> +#>
+#INSERT...ON DULPICATE KEY UPDATE> +#> +--echo # INSERT...ON DULPICATE
KEY UPDATESame as above. Typo too.> +CREATE TABLE ins_duplicate (id INT
PRIMARY KEY, val VARCHAR(1));> +INSERT INTO ins_duplicate VALUES
(1,'a');> +INSERT INTO ins_duplicate VALUES (2,'b') ON DUPLICATE KEY
UPDATE val='b' RETURNING id,val;> +INSERT INTO ins_duplicate VALUES
(3,'b') ON DUPLICATE KEY UPDATE val='c' RETURNING *;> +INSERT INTO
ins_duplicate VALUES (4,'b') ON DUPLICATE KEY UPDATE val='d' RETURNING
id AS id1;> +#> +# INSERT...SET> +#> +> +--echo # INSERT...SETSame as
above.> +INSERT INTO t1 SET id1 = 1, val1 = 'a'; > +INSERT INTO t1
SET id1 = 2, val1 = 'b' RETURNING *;> +INSERT INTO t1 SET id1 = 3,
val1 = 'c' RETURNING id1,val1;> +INSERT INTO t1 SET id1 = 3, val1 =
'c' RETURNING id1 AS id;> +> +DROP TABLE t1; > +DROP TABLE t2;> +DROP
TABLE ins_duplicate;> +> +#> +--echo #End of test case > +#This is
superfluous, please delete everything after the last DROP TABLE.> \ No
newline at end of file> diff --git a/mysql-
test/main/insert_parser_1.test b/mysql-test/main/insert_parser_1.test>
new file mode 100644> index 00000000000..f83c288a800> --- /dev/null>
+++ b/mysql-test/main/insert_parser_1.test> @@ -0,0 +1,75 @@Hmm, this
file does not have a corresponding .result file. It's almost a copyof
the previous one and it has windows line endings. It's the only one
thathas those. I suggest you move the statements not present in
insert_parser.testthere and delete this file entirely.> +#> +# Syntax
check for INSERT...RETURNING> +#> +> +--disable_warnings> +drop table
if exists t1,t2;> +--enable_warnings> +> +CREATE TABLE t1(id1 INT, val1
VARCHAR(1));> +--echo Table t1 created successfully. Fields: id1 INT,
val1 VARCHAR(1);> +> +CREATE TABLE t2(id2 INT, val2 VARCHAR(1));> +
--echo Table t2 created successfully. Fields: id2 INT, val2
VARCHAR(1);> +> +#> +#Simple insert statement> +#> +--echo #Simple
insert statement> +INSERT INTO t1 (id1, val1) VALUES (1, 'a');> +INSERT
INTO t1 (id1, val1) VALUES (2, 'b') RETURNING *;> +INSERT INTO t1 (id1,
val1) VALUES (3, 'c') RETURNING id1, val1;> +INSERT INTO t1 (id1, val1)
VALUES (4, 'd') RETURNING id1 as id;> +INSERT INTO t1 (id1, val1)
VALUES (5, 'e') RETURNING id1 && id1;> +INSERT INTO t1 (id1, val1)
VALUES (6, 'f') RETURNING id1 + id1;> +INSERT INTO t1 (id1, val1)
VALUES (7, 'g') RETURNING id1 | id1;> +SELECT * FROM t1;> +TRUNCATE
TABLE t1;> +SELECT * FROM t1;> +> +#> +#multiple values in one insert
statement> +#> +--echo #multiple values in one insert statement>
+INSERT INTO t1 VALUES (1,'a'),(2,'b');> +INSERT INTO t1 VALUES
(3,'c'),(4,'d') RETURNING *;> +INSERT INTO t1 VALUES (5,'e'),(6,'f')
RETURNING id1, val2;> +INSERT INTO t1 VALUES (7,'g'),(8,'h') RETURNING
id1 AS id;> +INSERT INTO t1 VALUES (9,'i'),(10,'j') RETURNING id1 | id1
;> +INSERT INTO t1 VALUES (11,'k'),(12,'l') RETURNING id1 + id1;>
+INSERT INTO t1 VALUES (13,'m'),(14,'n') RETURNING id1 && id1;> +SELECT
* FROM t1;> +> +#> +#INSERT...ON DULPICATE KEY UPDATE> +#> +--echo #
INSERT...ON DULPICATE KEY UPDATE> +CREATE TABLE ins_duplicate (id INT
PRIMARY KEY, val VARCHAR(1));> +INSERT INTO ins_duplicate VALUES
(1,'a');> +INSERT INTO ins_duplicate VALUES (2,'b') ON DUPLICATE KEY
UPDATE val='b' RETURNING id,val;> +INSERT INTO ins_duplicate VALUES
(2,'b') ON DUPLICATE KEY UPDATE val='c' RETURNING *;> +INSERT INTO
ins_duplicate VALUES (2,'b') ON DUPLICATE KEY UPDATE val='c' RETURNING
id AS id1;> +INSERT INTO ins_duplicate VALUES (2,'b') ON DUPLICATE KEY
UPDATE val='c' RETURNING id | id;> +INSERT INTO ins_duplicate VALUES
(2,'b') ON DUPLICATE KEY UPDATE val='c' RETURNING id && id;> +INSERT
INTO ins_duplicate VALUES (2,'b') ON DUPLICATE KEY UPDATE val='c'
RETURNING id +id;> +SELECT * FROM ins_duplicate;> +> +#> +#
INSERT...SET> +#> +--echo # INSERT...SET> +INSERT INTO t1 SET id1 = 1,
val1 = 'a'; > +INSERT INTO t1 SET id1 = 2, val1 = 'b' RETURNING *;>
+INSERT INTO t1 SET id1 = 3, val1 = 'c' RETURNING id1,val1;> +INSERT
INTO t1 SET id1 = 3, val1 = 'c' RETURNING id1 as id;> +INSERT INTO t1
SET id1 = 3, val1 = 'c' RETURNING id1 | id1;> +INSERT INTO t1 SET id1
= 3, val1 = 'c' RETURNING id1 && id1;> +INSERT INTO t1 SET id1 = 3,
val1 = 'c' RETURNING id1 + id1;> +> +DROP TABLE t1; > +DROP TABLE t2;>
+DROP TABLE ins_duplicate;> +> +#> +--echo #End of test case > +#> \ No
newline at end of file> diff --git a/mysql-
test/main/insert_parser_sel.test b/mysql-
test/main/insert_parser_sel.test> new file mode 100644> index
00000000000..f144cfc48ec> --- /dev/null> +++ b/mysql-
test/main/insert_parser_sel.test> @@ -0,0 +1,45 @@> +#> +# Syntax check
for INSERT...RETURNING> +#> +> +--disable_warnings> +drop table if
exists t1,t2;> +--enable_warningsSimilar to insert_parser.test, these
lines are not necessary.> +> +CREATE TABLE t1(id1 INT, val1
VARCHAR(1));> +--echo Table t1 created successfully. Fields: id1 INT,
val1 VARCHAR(1);See insert_parser.test comments.> +> +CREATE TABLE
t2(id2 INT, val2 VARCHAR(1));> +--echo Table t2 created successfully.
Fields: id2 INT, val2 VARCHAR(1);See insert_parser.test comments.> +>
+#> +#Simple insert statement> +#> +--echo #Simple insert statementSee
insert_parser.test comments.> +INSERT INTO t1 (id1, val1) VALUES (1,
'a');> +INSERT INTO t1 (id1, val1) VALUES (2, 'b');> +INSERT INTO t1
(id1, val1) VALUES (3, 'c');> +INSERT INTO t1 (id1, val1) VALUES (4,
'd');> +INSERT INTO t1 (id1, val1) VALUES (5, 'e');> +INSERT INTO t1
(id1, val1) VALUES (6, 'f');> +INSERT INTO t1 (id1, val1) VALUES (7,
'g');> +SELECT * FROM t1;> +TRUNCATE TABLE t1;> +SELECT * FROM t1;> +>
+#> +# INSERT...SELECT> +#> +--echo #INSERT...SELECTSee
insert_parser.test comments.> +INSERT INTO t2(id2,val2) SELECT * FROM
t1;> +INSERT INTO t2(id2,val2) SELECT * FROM t1 WHERE id1=1 RETURNING
*;> +INSERT INTO t2(id2,val2) SELECT * FROM t1 WHERE id1=2 RETURNING
id2,val2;> +INSERT INTO t2(id2,val2) SELECT * FROM t1 WHERE id1=3
RETURNING id2 AS id;> +INSERT INTO t2(id2,val2) SELECT * FROM t1 WHERE
id1 IN (SELECT id1 FROM t1 WHERE id1 IN (3,4,5) );Ok, this last line
does not test RETURNING at all, what was the intent?Also, please wrap
queries to maximum 80 lines. Find the right way to indentthe SQL query
on multiple lines to make it easier to read.> +SELECT * FROM t2;> +DROP
TABLE t1; > +DROP TABLE t2;> +> +#> +--echo #End of test case > +#See
insert_parser.test comments.> \ No newline at end of file> diff --git
a/mysql-test/main/insert_returning_complex.test b/mysql-
test/main/insert_returning_complex.test> new file mode 100644> index
00000000000..59ef3342e76> --- /dev/null> +++ b/mysql-
test/main/insert_returning_complex.test> @@ -0,0 +1,117 @@> +#> +# Test
for INSERT...RETURNING with virual cols,operators and # subqueries> +#>
+> +--disable_warnings> +drop table if exists t1,t2,ins_duplicate;> +
--enable_warningsSee insert_returning.testI would merge this test into
the insert_returning.test file. No need to split them up.> +> +CREATE
TABLE t1(id1t1 INT, id2t1 INT, valt1 VARCHAR(1));> +CREATE TABLE
t2(id1t2 INT, id2t2 INT, valt2 VARCHAR(1));> +INSERT INTO t2 VALUES
(1,2,'a'),(2,3,'b'),(3,4,'c'),(4,5,'d'),(5,6,'e'),(6,7,'f'),(7,8,'g'),(
8,9,'h'),(9,10,'i'),(10,11,'j');Please wrap this line to 80 characters
max.> +> +--echo Table t1 created successfully. Fields: id1 INT, id2
INT, val VARCHAR(1);Superfluous.> +> +#> +#Simple insert statement> +#>
+--echo #Simple insert statementSee insert_returning.test comments.>
+INSERT INTO t1 (id1t1, id2t1, valt1) VALUES (1, 2, 'a');> +INSERT INTO
t1 (id1t1, id2t1, valt1) VALUES (2, 3, 'b') RETURNING *;> +INSERT INTO
t1 (id1t1, id2t1, valt1) VALUES (3, 4, 'c') RETURNING id1t1, valt1;>
+INSERT INTO t1 (id1t1, id2t1, valt1) VALUES (4, 5, 'd') RETURNING
id1t1 AS id;> +INSERT INTO t1 (id1t1, id2t1, valt1) VALUES (5, 6, 'e')
RETURNING id1t1+id2t1 AS total;> +INSERT INTO t1 (id1t1, id2t1, valt1)
VALUES (6, 7, 'f') RETURNING id1t1 & id2t1;> +INSERT INTO t1 (id1t1,
id2t1, valt1) VALUES (7, 8, 'g') RETURNING id1t1 || id2t1;> +INSERT
INTO t1 (id1t1, id2t1, valt1) VALUES (8,9,'h') RETURNING
id1t1,UPPER(valt1);> +INSERT INTO t1(id1t1,id2t1,valt1) VALUES
(9,10,'i') RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 WHERE
id1t2=1);> +INSERT INTO t1(id1t1,id2t1,valt1) VALUES(10,11,'j')
RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 GROUP BY id1t2 HAVING
id1t2=id1t2+1);Ok, please wrap all such lines to 80 characters to make
them easier to parse.Second, can we test with subqueries that return
multiple rows, for example:SELECT GROUP_CONCAT(valt2) FROM t2 group by
id1t2.We should return an error here, but this should be covered in the
test casetoo.> +INSERT INTO t1 (id1t1, id2t1, valt1) VALUES (8, 9, 'h')
RETURNING id1t1,(SELECT id2t2 FROM t2 WHERE valt2='b');Can you also
test a subquery that uses columns from t1?Example:INSERT INTO t1
(id1t1, id2t1, valt1) VALUES (8, 9, 'h') RETURNING id1t1, (SELECT
id2t2 + id1t1 FROM t2 WHERE valt2='b');> > +SELECT * FROM t1;>
+TRUNCATE TABLE t1;> +SELECT * FROM t1;> +> +> +#> +#multiple values in
one insert statement> +#> +--echo #multiple values in one insert
statementSee insert_returning.test comments.> +INSERT INTO t1 VALUES
(1, 2, 'a'),(2, 3, 'b');> +INSERT INTO t1 VALUES (3, 4, 'c'),(4, 5,
'd') RETURNING *;> +INSERT INTO t1 VALUES (5, 6, 'e'),(6, 7, 'f')
RETURNING id2t1, valt1;> +INSERT INTO t1 VALUES (7, 8, 'g'),(8, 9, 'h')
RETURNING id2t1 AS id;> +INSERT INTO t1 VALUES (9, 10, 'i'),(10, 11,
'j') RETURNING id2t1+id1t1 as total;> +INSERT INTO t1 VALUES (11, 12,
'k'),(12, 13, 'l') RETURNING id2t1 & id1t1;> +INSERT INTO t1 VALUES (5,
6, 'e'),(2, 3, 'f') RETURNING id2t1 || id1t1;> +INSERT INTO t1 VALUES
(13,14,'m'),(14,15,'n') RETURNING id1t1,UPPER(valt1);> +INSERT INTO t1
VALUES (15,16,'o'),(16,17,'p') RETURNING (SELECT GROUP_CONCAT(valt2)
FROM t2 WHERE id1t2=1);> +INSERT INTO t1 VALUES (17,18,'q'),(18,19,'r')
RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 GROUP BY id1t2 HAVING
id1t2=id1t2+1)> +INSERT INTO t1 VALUES (19,20,'r'),(20,21,'s')
RETURNING id1t1,(SELECT id2t2 FROM t2 WHERE valt2='b');Same comments
as for single-value insert.> +SELECT * FROM t1;> +TRUNCATE TABLE t1;>
+> +#> +#INSERT...ON DULPICATE KEY UPDATE> +#> +--echo # INSERT...ON
DULPICATE KEY UPDATETypo DULPICATE -> DUPLICATE. Also, see comments
from insert_returning.test.> +CREATE TABLE ins_duplicate (id1 INT
PRIMARY KEY, id2 INT, val VARCHAR(1));> +INSERT INTO ins_duplicate
VALUES (1,2,'a');> +INSERT INTO ins_duplicate VALUES (2,3,'b') ON
DUPLICATE KEY UPDATE val='b' RETURNING id1,val;> +INSERT INTO
ins_duplicate VALUES (2,4,'b') ON DUPLICATE KEY UPDATE val='c'
RETURNING *;> +INSERT INTO ins_duplicate VALUES (2,5,'b') ON DUPLICATE
KEY UPDATE val='d' RETURNING id1+id2 AS total;> +INSERT INTO
ins_duplicate VALUES (2,6,'b') ON DUPLICATE KEY UPDATE val='e'
RETURNING id1 || id2;> +INSERT INTO ins_duplicate VALUES (2,7,'b') ON
DUPLICATE KEY UPDATE val='f' RETURNING id1 & id2;> +INSERT INTO
ins_duplicate VALUES (2,8,'b') ON DUPLICATE KEY UPDATE val='g'
RETURNING id1 AS id;> +INSERT INTO ins_duplicate VALUES (2,9,'b') ON
DUPLICATE KEY UPDATE val='h' RETURNING id1,UPPER(val);> +INSERT INTO
ins_duplicate VALUES (2,10,'b') ON DUPLICATE KEY UPDATE val='i'
RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 WHERE id1t2=1);> +INSERT
INTO ins_duplicate VALUES (2,11,'b') ON DUPLICATE KEY UPDATE val='j'
RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 GROUP BY id1t2 HAVING
id1t2=id1t2+1);> +INSERT INTO ins_duplicate VALUES (2,12,'b') ON
DUPLICATE KEY UPDATE val='k' RETURNING id1,(SELECT id2t2 FROM t2 WHERE
valt2='b');> +SELECT * FROM ins_duplicate;> +> +#> +# INSERT...SET> +#>
+> +--echo # INSERT...SET> +INSERT INTO t1 SET id1t1=1, id2t1=2,
valt1='a'; > +INSERT INTO t1 SET id1t1=2, id2t1=3, valt1='b' RETURNING
*;> +INSERT INTO t1 SET id1t1=3, id2t1=4, valt1='c' RETURNING valt1;>
+INSERT INTO t1 SET id1t1=4, id2t1=5, valt1='d' RETURNING id1t1+id2t1
AS total;> +INSERT INTO t1 SET id1t1=5, id2t1=6, valt1='e' RETURNING
id1t1 & id2t1;> +INSERT INTO t1 SET id1t1=6, id2t1=7, valt1='f'
RETURNING id1t1 || id2t1;> +INSERT INTO t1 SET id1t1=7, id2t1=8,
valt1='g' RETURNING valt1 AS letter;> +INSERT INTO t1 SET id1t1=8,
id2t1=9, valt1='h' RETURNING id1t1,UPPER(valt1);> +INSERT INTO t1 SET
id1t1=9, id2t1=10, valt1='i' RETURNING (SELECT GROUP_CONCAT(valt2) FROM
t2 WHERE id1t2=1);> +INSERT INTO t1 SET id1t1=10, id2t1=11, valt1='j'
RETURNING (SELECT GROUP_CONCAT(valt2) FROM t2 GROUP BY id1t2 HAVING
id1t2=id1t2+1);> +INSERT INTO t1 SET id1t1=11, id2t1=12, valt1='k'
RETURNING id1t1,(SELECT id2t2 FROM t2 WHERE valt2='b'); > +SELECT *
FROM t1;> +TRUNCATE TABLE t1;> +SELECT * FROM t1;> +> +#> +#
INSERT...SELECT> +#> +--echo # INSERT...SELECT> +INSERT INTO t1(id1t1,
id2t1, valt1) SELECT * FROM t2 WHERE id1t2=1;> +INSERT INTO t1(id1t1,
id2t1, valt1) SELECT * FROM t2 WHERE id1t2=2 RETURNING *;> +INSERT INTO
t1(id1t1, id2t1, valt1) SELECT * FROM t2 WHERE id1t2=3 RETURNING
valt1;> +INSERT INTO t1(id1t1, id2t1, valt1) SELECT * FROM t2 WHERE
id1t2=4 RETURNING id1t1+id2t1 AS total;> +INSERT INTO t1(id1t1, id2t1,
valt1) SELECT * FROM t2 WHERE id1t2=5 RETURNING id1t1 & id2t1;> +INSERT
INTO t1(id1t1, id2t1, valt1) SELECT * FROM t2 WHERE id1t2=6 RETURNING
valt1 AS letter;> +INSERT INTO t1(id1t1, id2t1, valt1) SELECT * FROM t2
WHERE id1t2=2 RETURNING id1t1 || id2t1;> +INSERT INTO t1(id1t1, id2t1,
valt1) SELECT * FROM t2 WHERE id1t2=7 RETURNING id1t1,UPPER(valt1);>
+INSERT INTO t1(id1t1, id2t1, valt1) SELECT * FROM t2 WHERE id1t2=8>
+RETURNING (SELECT GROUP_CONCAT(val) FROM ins_duplicate WHERE id1=1);>
+INSERT INTO t1(id1t1, id2t1, valt1) SELECT * FROM t2 WHERE id1t2=9>
+RETURNING (SELECT GROUP_CONCAT(val) FROM ins_duplicate GROUP BY id1
HAVING id1=id1+1);> +INSERT INTO t1(id1t1, id2t1, valt1) SELECT * FROM
t2 WHERE id1t2=10 RETURNING id1t1,(SELECT id2 FROM ins_duplicate WHERE
valt2='b');> +SELECT* FROM t1;> +> +--echo Droping t1 and
ins_duplicate> +DROP TABLE t1;> +DROP TABLE t2; > +DROP TABLE
ins_duplicate;> +> +#> +--echo #End of test case > +# > \ No newline at
end of file> Binary files /dev/null and b/mysql-
test/main/insert_returning_datatypes.result differOk, why is this a
binary file for git? Something went wrong here.> diff --git a/mysql-
test/main/insert_returning_datatypes.test b/mysql-
test/main/insert_returning_datatypes.test> new file mode 100644> index
00000000000..803c8326b6f> --- /dev/null> +++ b/mysql-
test/main/insert_returning_datatypes.test> @@ -0,0 +1,92 @@> +#>
+# Tests for DELETE FROM <table> ... RETURNING <expr>,...> +# What is
DELETE FROM .. RETURNING doing here? :)> +> +--disable_warnings> +drop
table if exists t1,t2;> +--enable_warningsI vote for merging this into
the previous tests.> +> +CREATE TABLE t1(num_int1 INT(2) PRIMARY KEY, >
+ num_bit1 BIT(2),> + num_float1
FLOAT(5,2),> + num_double1 DOUBLE(5,2),>
+ char_enum1 ENUM('A','B','C','D'),>
+ char_set1 SET('a','b','c','d','e'),>
+ str_varchar1 VARCHAR(2),> + str_binary1
BINARY(2),> + d1 DATE,> + dt1 DATETIME,>
+ ts1 TIMESTAMP,> + y1 YEAR,>
+ b1 BOOL);> +> +CREATE TABLE t2(num_int2 INT(2) PRIMARY
KEY, > + num_bit2 BIT(2),> + num_float2
FLOAT(5,2),> + num_double2 DOUBLE(5,2),>
+ char_enum2 ENUM('A','B','C','D'),>
+ char_set2 SET('a','b','c','d','e'),>
+ str_varchar2 VARCHAR(2),> + str_binary2
BINARY(2),> + d2 DATE,> + dt2 DATETIME,>
+ ts2 TIMESTAMP,> + y2 YEAR,>
+ b2 BOOL);> +> +> +#> +# SIMLPE INSERT STATEMENT>
+#Typo, but please delete this anayway.> +--echo # SIMPLE INSERT
STATEMENTSee other test comments.> +INSERT INTO
t1(num_int1,num_bit1,num_float1,num_double1,char_enum1,char_set1,str_va
rchar1, str_binary1,d1,dt1,ts1,y1,b1) VALUES(1,0, 123.45, 123.55,
'A','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0) RETURNING *;> +> +INSERT INTO
t1(num_int1,num_bit1,num_float1,num_double1,char_enum1,char_set1,str_va
rchar1, str_binary1,d1,dt1,ts1,y1,b1) VALUES(2,0, 123.45, 123.55,
'A','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0) RETURNING num_int1,num_bit1,d1,dt1;> +> +> +> +#> +#
MULTIPLE ROWS IN SINGLE STATEMENT > +#> +--echo #Multiple rows inserted
in one statement> +INSERT INTO t1 VALUES(3,0, 123.45, 123.55,
'A','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0),(4,0, 123.45, 123.55, 'A','b,e',
'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,1),(5,0, 123.45, 123.55,
'A','b,e','V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,1) RETURNING *;> +> +INSERT INTO t1 VALUES(6,0, 123.45,
123.55, 'A','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0),(7,0, 123.45, 123.55, 'A','b,e',
'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,1),(8,0, 123.45, 123.55,
'A','b,e','V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,1) RETURNING char_set1,ts1,y1;> +> +> +> +#>
+#INSERT...SET...RETURNING> +#> +--echo #INSERT...SET...RETURNING>
+INSERT INTO t1 SET
num_int1=9,num_bit1=0,num_float1=124.67,num_double1=231.12,char_enum1='
B',char_set1='a,d,e',str_varchar1='AB',str_binary1='01',
d1='120314',dt1="2012-04-19 13:08:22",ts1='2001-07-22
12:12:1',y1=2014,b1=1 RETURNING *;> +> +INSERT INTO t1 SET
num_int1=10,num_bit1=0,num_float1=124.67,num_double1=231.12,char_enum1=
'B',char_set1='a,d,e',str_varchar1='AB',str_binary1='01',d1='120314',dt
1="2012-04-19 13:08:22",ts1='2001-07-22 12:12:1',y1=2014,b1=1 RETURNING
b1,char_enum1;> +> +> +> +#> +# INSERT...ON DUPLICATE KEY UPDATE> +# >
+--echo # INSERT...ON DUPLCATE KEY UPDATE...RETURNING> +INSERT INTO
t1(num_int1,num_bit1,num_float1,num_double1,char_enum1,char_set1,str_va
rchar1, str_binary1,d1,dt1,ts1,y1,b1) VALUES (10,0, 123.45, 123.55,
'C','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0) ON DUPLICATE KEY UPDATE num_float1=111.111 RETURNING
*;> +> +INSERT INTO
t1(num_int1,num_bit1,num_float1,num_double1,char_enum1,char_set1,str_va
rchar1, str_binary1,d1,dt1,ts1,y1,b1) VALUES (11,0, 123.45, 123.55,
'A','b,e', 'V','10','120314',"2012-04-19 13:08:22", '2001-07-22
12:12:12',2012,0) ON DUPLICATE KEY UPDATE char_set1='a,b' RETURNING
char_set1,num_int1;> +> +> +> +#> +# INSERT...SELECT...RETURNING> +#>
+--echo # INSERT...SELECT...RETURNING> +INSERT INTO t2
(num_int2,num_bit2,num_float2,num_double2,char_enum2,char_set2,str_varc
har2, str_binary2,d2,dt2,ts2,y2,b2) SELECT * FROM t1 RETURNING *;>
+TRUNCATE TABLE t2;> +INSERT INTO t2
(num_int2,num_bit2,num_float2,num_double2,char_enum2,char_set2,str_varc
har2, str_binary2,d2,dt2,ts2,y2,b2) SELECT * FROM t1 RETURNING
num_double2,str_varchar2,str_binary2;> +> +> +DROP TABLE t1;> +DROP
TABLE t2;> +> +#> +#END OF TEST CASE> +# > \ No newline at end of file>
diff --git a/mysql-test/main/insert_returning_err.test b/mysql-
test/main/insert_returning_err.test> new file mode 100644> index
00000000000..4af798e5c68> --- /dev/null> +++ b/mysql-
test/main/insert_returning_err.test> @@ -0,0 +1,97 @@> +#> +# Test for
checking error message for INSERT...RETURNING> +#> +> +#INSERT INTO
<table> ... RETURNING <not existing col>> +#INSERT INTO <table> ...
RETURNING <expr with aggr function>> +#INSERT INTO ... RETURNING
subquery with more than 1 row> +#INSERT INTO ... RETURNING operand
should contain 1 colunm(s)> +#INSERT INTO ... RETURNING operand should
contain 1 colunm(s)> +> +--disable_warnings> +drop table if exists
t1,t2;> +--enable_warnings> +> +CREATE TABLE t1(id1 INT,val1
VARCHAR(1));> +CREATE TABLE t2(id2 INT,val2 VARCHAR(1));> +CREATE TABLE
ins_duplicate (id INT PRIMARY KEY, val VARCHAR(1));> +> +INSERT INTO t1
VALUES(1,'a'),(2,'b'),(3,'c');> +> +#> +# SIMLPE INSERT STATEMENT> +#>
+--error ER_BAD_FIELD_ERROR> +INSERT INTO t2(id2,val2) VALUES(1,'a')
RETURNING id1;> +--error ER_INVALID_GROUP_FUNC_USE> +INSERT INTO
t2(id2,val2) values(2,'b') RETURNING MAX(id2);> +--error
ER_SUBQUERY_NO_1_ROW> +INSERT INTO t2(id2,val2) VALUES(3,'c') RETURNING
(SELECT id1 FROM t1);Ok, so you do test this, good. What about a
subquery using a column from t2?> +--error ER_OPERAND_COLUMNS> +INSERT
INTO t2(id2,val2) VALUES(4,'d') RETURNING (SELECT * FROM t1);This query
fails specifically because there are multiple columns in the
subselect.What happens if there is only one column?> +--error
ER_OPERAND_COLUMNS> +INSERT INTO t2(id2,val2) VALUES(4,'d') RETURNING
(SELECT * FROM t2);> +> +#> +# Multiple rows in single insert
statement> +#> +--error ER_BAD_FIELD_ERROR> +INSERT INTO t2
VALUES(1,'a'),(2,'b') RETURNING id1;> +--error
ER_INVALID_GROUP_FUNC_USE> +INSERT INTO t2 VALUES(3,'c'),(4,'d')
RETURNING MAX(id2);> +--error ER_SUBQUERY_NO_1_ROW> +INSERT INTO t2
VALUES(5,'c'),(6,'f') RETURNING (SELECT id1 FROM t1);> +--error
ER_OPERAND_COLUMNS> +INSERT INTO t2 VALUES(7,'g'),(8,'h') RETURNING
(SELECT * FROM t1);> +--error ER_OPERAND_COLUMNS> +INSERT INTO t2
VALUES(7,'g'),(8,'h') RETURNING (SELECT * FROM t2);> +> +#> +# INSERT
... SET> +#> +--error ER_BAD_FIELD_ERROR> +INSERT INTO t2 SET id2=1,
val2='a' RETURNING id1;> +--error ER_INVALID_GROUP_FUNC_USE> +INSERT
INTO t2 SET id2=2, val2='b' RETURNING MAX(id2);> +--error
ER_SUBQUERY_NO_1_ROW> +INSERT INTO t2 SET id2=3, val2='c' RETURNING
(SELECT id1 FROM t1);> +--error ER_OPERAND_COLUMNS> +INSERT INTO t2 SET
id2=4, val2='d' RETURNING (SELECT * FROM t1);> +--error
ER_OPERAND_COLUMNS> +INSERT INTO t2 SET id2=4, val2='d' RETURNING
(SELECT * FROM t2);> +> +#> +#INSERT...ON DUPLICATE KEY UPDATE> +#> +
--error ER_BAD_FIELD_ERROR> +INSERT INTO ins_duplicate VALUES (2,'b')
ON DUPLICATE KEY UPDATE val='b' RETURNING id1;> +--error
ER_INVALID_GROUP_FUNC_USE> +INSERT INTO ins_duplicate VALUES (2,'b') ON
DUPLICATE KEY UPDATE val='b' RETURNING MAX(id);> +--error
ER_SUBQUERY_NO_1_ROW> +INSERT INTO ins_duplicate VALUES (2,'b') ON
DUPLICATE KEY UPDATE val='b' RETURNING (SELECT id1 FROM t1);> +--error
ER_OPERAND_COLUMNS> +INSERT INTO ins_duplicate VALUES (2,'b') ON
DUPLICATE KEY UPDATE val='b' RETURNING (SELECT * FROM t1);> +--error
ER_OPERAND_COLUMNS> +INSERT INTO ins_duplicate VALUES (2,'b') ON
DUPLICATE KEY UPDATE val='b' RETURNING (SELECT * FROM ins_duplicate);>
+> +#> +#INSERT...SELECT...RETURNING> +#Use --echo # for the lines
above please.> +--error ER_BAD_FIELD_ERROR> +INSERT INTO t2(id2, val2)
SELECT * FROM t1 WHERE id1=1 RETURNING id1;> +--error
ER_INVALID_GROUP_FUNC_USE> +INSERT INTO t2(id2, val2) SELECT * FROM t1
WHERE id1=2 RETURNING MAX(id2);> +--error ER_SUBQUERY_NO_1_ROW> +INSERT
INTO t2(id2, val2) SELECT * FROM t1 WHERE id1=2 RETURNING (SELECT id1
FROM t1);> +--error ER_OPERAND_COLUMNS> +INSERT INTO t2(id2, val2)
SELECT * FROM t1 WHERE id1=2 RETURNING (SELECT * FROM t1);> +--error
ER_OPERAND_COLUMNS> +INSERT INTO t2(id2, val2) SELECT * FROM t1 WHERE
id1=2 RETURNING (SELECT * FROM t2);> +> +DROP TABLE t1;> +DROP TABLE
t2;> +DROP TABLE ins_duplicate;> +> +#> +# End of test case> +# > \ No
newline at end of file> diff --git a/mysql-
test/main/insert_returning_stmt.result b/mysql-
test/main/insert_returning_stmt.result> new file mode 100644> index
00000000000..3b48cb11b28> --- /dev/null> +++ b/mysql-
test/main/insert_returning_stmt.resultHow is this test different than
previous ones? I am getting a bit confusedby the large number of test
files that seem simillar.> \ No newline at end of file> diff --git
a/mysql-test/main/insert_sel_returning.test b/mysql-
test/main/insert_sel_returning.test> new file mode 100644> index
00000000000..8b4880e2064> --- /dev/null> +++ b/mysql-
test/main/insert_sel_returning.test> diff --git a/sql/sql_class.h
b/sql/sql_class.h> index 18ccf992d1e..8e67818f891 100644> ---
a/sql/sql_class.h> +++ b/sql/sql_class.h> @@ -5475,13 +5475,15 @@ class
select_dump :public select_to_file {> > class select_insert :public
select_result_interceptor {> public:> + select_result* sel_result;>
+ bool with_returning_list;> TABLE_LIST *table_list;> TABLE
*table;> List<Item> *fields;> ulonglong
autoinc_value_of_last_inserted_row; // autogenerated or
not> COPY_INFO info;> bool insert_into_view;>
- select_insert(THD *thd_arg, TABLE_LIST *table_list_par,>
+ select_insert(bool with_ret_list, select_result* sel_ret_list, THD
*thd_arg, TABLE_LIST *table_list_par,When adding parameters, please add
them to the end of the function, not thestart. Do we need both
with_returning_list and sel_result pointer? Can wenot use just the
sel_result pointer and if it is null, we assume we do nothave a
returning list?> TABLE *table_par, List<Item>
*fields_par,> List<Item> *update_fields, List<Item>
*update_values,> enum_duplicates duplic, bool ignore);>
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc> index
f3a548d7265..6eb9883fe87 100644> --- a/sql/sql_insert.cc> +++
b/sql/sql_insert.cc> @@ -1236,26 +1261,38 @@ bool mysql_insert(THD
*thd,TABLE_LIST *table_list,> if ((iteration * values_list.elements)
== 1 && (!(thd->variables.option_bits & OPTION_WARNINGS) ||>
!thd->cuted_fields))> {> - my_ok(thd,
info.copied + info.deleted +> - ((thd-
>client_capabilities & CLIENT_FOUND_ROWS) ?>
- info.touched : info.updated),> - id);> +
if (with_returning_list)> + result->send_eof();> +
else > + {> + my_ok(thd, info.copied + info.deleted
+> + ((thd->client_capabilities &
CLIENT_FOUND_ROWS) ?> + info.touched :
info.updated),> + id);> + }> }> e
lse> {> char buff[160];> ha_rows updated=((thd-
>client_capabilities & CLIENT_FOUND_ROWS)
?> info.touched : info.updated);> - if
(ignore)> - sprintf(buff, ER_THD(thd, ER_INSERT_INFO), (ulong)
info.records,> - (lock_type == TL_WRITE_DELAYED) ? (ulong)
0 :> - (ulong) (info.records - info.copied),>
- (long) thd->get_stmt_da()-
>current_statement_warn_count());> - else> - sprintf(buff,
ER_THD(thd, ER_INSERT_INFO), (ulong) info.records,> - (ulong)
(info.deleted + updated),> - (long) thd->get_stmt_da()-
>current_statement_warn_count());> - ::my_ok(thd, info.copied +
info.deleted + updated, id, buff);> + if (ignore)> + sprintf
(buff, ER_THD(thd, ER_INSERT_INFO), (ulong)info.records,> +
(lock_type == TL_WRITE_DELAYED) ? (ulong)0 :> + (ulong)
(info.records - info.copied),> + (long)thd-
>get_stmt_da()->current_statement_warn_count());> + else > +
{> + if (with_returning_list)> + result-
>send_eof();> + else > + {> +
sprintf(buff, ER_THD(thd, ER_INSERT_INFO), (ulong)info.records,> +
(ulong)(info.deleted + updated),> +
(long)thd->get_stmt_da()-
>current_statement_warn_count());> + ::my_ok(thd,
info.copied + info.deleted + updated, id, buff);> + }> +
}Uhm, careful here, you have changed the logic of when my_ok gets
called!
Please re-read this bit of code and try to understand where things
havechanged. Also, looking at this, can you test INSERT IGNORE ...
RETURNING too?
I have a feeling it won't work with the current
implementation.> }> thd->abort_on_warning= 0;> if (thd->lex-
>current_select->first_cond_optimization)> @@ -1557,7 +1595,10 @@ bool
mysql_prepare_insert(THD *thd, TABLE_LIST
*table_list,> table_list->next_local= 0;> context-
>resolve_in_table_list_only(table_list);> > - res=
(setup_fields(thd, Ref_ptr_array(),> + res=
((with_returning_list?((sel_lex->with_wild && setup_wild(thd,
table_list, sel_lex->returning_list, NULL, sel_lex->with_wild,> +
&select_lex->hidden_bit_fields)) ||> + setup_fields(th
d, Ref_ptr_array(),> + sel_lex->returning_list,
MARK_COLUMNS_READ, 0, NULL, 0)):false)||setup_fields(thd,
Ref_ptr_array(),Uhm, please fix the formatting for this code. The logic
is also quite convoluted,can you turn this into something actually
readable?
Also, you are calling the same setup_wild && setup_fields call both
withinmysql_prepare_insert, but also within select_result->prepare. Why
2 times?> *values, MARK_COLUMNS_READ, 0, NULL,
0) ||> check_insert_fields(thd, context->table_list, fields,
*values,> !insert_into_view, 0, &map));>
@@ -3557,12 +3598,13 @@ bool
Delayed_insert::handle_inserts(void)> TRUE Error> */> > -bool
mysql_insert_select_prepare(THD *thd)> +bool
mysql_insert_select_prepare(THD *thd,select_result
*sel_res)> {> LEX *lex= thd->lex;> SELECT_LEX *select_lex= lex-
>first_select_lex();> DBUG_ENTER("mysql_insert_select_prepare");> ->
+ bool with_ret = lex->current_select-
>returning_list.is_empty()?false:true;> + List<Item>& returning_list =
thd->lex->current_select->returning_list;Not really a fan of this
method. By doing this, it's going to be impossibleto extend INSERT ..
RETURNING to work within subqueries.> > /*> SELECT_LEX do not
belong to INSERT statement, so we can't add WHERE> @@ -3572,14 +3614,18
@@ bool mysql_insert_select_prepare(THD *thd)> if
(mysql_prepare_insert(thd, lex-
>query_tables,> lex->query_tables->table,
lex->field_list, 0,> lex->update_list, lex-
>value_list, lex->duplicates,> - &select_lex-
>where, TRUE))> + &select_lex->where,
TRUE,with_ret?select_lex:NULL))I don't like that we end up passing
select_lex to mysql_prepare_insert.Can we make the arguments of this
function be strictly what is necessary,not the full SELECT_LEX
structure?> DBUG_RETURN(TRUE);> > + if (with_ret)> + (void
)sel_res->prepare(returning_list, NULL);> +> DBUG_ASSERT(select_lex-
>leaf_tables.elements != 0);> List_iterator<TABLE_LIST>
ti(select_lex->leaf_tables);> TABLE_LIST *table;> uint
insert_tables;> > +> if (select_lex-
>first_cond_optimization)> {> /* Back up leaf_tables list. */>
@@ -3630,6 +3677,8 @@ select_insert::select_insert(THD *thd_arg,
TABLE_LIST *table_list_par,> info.update_values=
update_values;> info.view= (table_list_par->view ? table_list_par :
0);> info.table_list= table_list_par;> + sel_result= result;>
+ with_returning_list= with_ret_list;Can we get rid of
with_returning_list and just use sel_result? If it's
null,with_returning_list should also be null.> }> > > @@ -3651,7
+3701,28 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT
*u)> */> lex->current_select= lex->first_select_lex();> >
- res= (setup_fields(thd, Ref_ptr_array(),> + /*> + We want the
returning_list to point to insert table. But the context is masked. > +
So we swap it with the context saved during parsing stage. > + */>
+ if(with_returning_list) > + { swap_context(select_lex-
>table_list.saved_first,select_lex->table_list.first);>
+ swap_context(select_lex->context.saved_table_list,select_lex-
>context.table_list);> + swap_context(select_lex-
>context.saved_name_resolution_table,select_lex-
>context.first_name_resolution_table);> +> + res=((select_lex-
>with_wild && setup_wild(thd, table_list, select_lex->returning_list,
NULL, select_lex->with_wild,> + &select_lex-
>hidden_bit_fields)) ||> + setup_fields(thd,
Ref_ptr_array(),> + select_lex->returning_list,
MARK_COLUMNS_READ, 0, NULL, 0));> +> + /*Swap it back to
retore the previous state for the rest of the function*/> +> +
swap_context(select_lex->table_list.saved_first,select_lex-
>table_list.first);> + swap_context(select_lex-
>context.saved_table_list,select_lex->context.table_list);> +
swap_context(select_lex->context.saved_name_resolution_table,
select_lex->context.first_name_resolution_table);> + }I am not sure
this will work if you have columns with the same names, bothin t1 and
t2 (your tests don't cover this).Ex:CREATE table t1 (pk int);CREATE
table t2 (pk int);INSERT INTO t2 (pk) values (1), (2);
INSERT INTO t1 SELECT * FROM t2 RETURNING pk;
Also, what about INSERT INTO t1 SELECT .. FROM t1? Please add a test
casefor this and see if the code still continues to work.
Also, the saved_first introduction into SQL_I_List is really a hack.One
needs to save the correct list of tables separately, properly.
Pleasefind a solution without changing SQL_I_List. If this part gets
you stuck,we should discuss it, but I want you first to try and find a
proper solutionfirst.> +> + res= res || (setup_fields(thd,
Ref_ptr_array(),> values, MARK_COLUMNS_READ, 0,
NULL, 0) ||> check_insert_fields(thd, table_list, *fields,
values,> !insert_into_view, 1, &map));> @@
-4046,7 +4136,10 @@ bool select_insert::send_ok_packet() {> thd-
>first_successful_insert_id_in_prev_stmt :> (info.copied ?
autoinc_value_of_last_inserted_row : 0));> > - ::my_ok(thd,
row_count, id, message);> + if (with_returning_list)> + sel_r
esult->send_eof();> + else> + ::my_ok(thd, row_count, id,
message);I believe you need to return my_ok
regardless.> > DBUG_RETURN(false);> }> diff --git
a/sql/sql_parse.cc b/sql/sql_parse.cc> index 01d0ed1c383..6cee252096b
100644> --- a/sql/sql_parse.cc> +++ b/sql/sql_parse.cc> @@ -4604,11
+4664,21 @@ mysql_execute_command(THD *thd)> TODO: fix it by
removing the front element (restoring of it should> be done
properly as well)> */> + /*> + If items are present in
returning_list, then we need those items to point to > +
INSERT table during setup_fields() and setup_wild(). But it gets masked
before that. > + So we save the values in saved_first,
saved_table_list and saved_first_name_resolution_context.> +
before they are masked.> + */> + select_lex-
>table_list.saved_first=select_lex->table_list.first;Right, this is a
hack. Please check my other comment about adding saved_first.Please
find a way to get the correct list of tables for name resolutionwithout
extending SQL_I_List.> select_lex->table_list.first=
second_table;> + select_lex-
>context.saved_table_list=select_lex->context.table_list;> + selec
t_lex->context.saved_name_resolution_table=> + selec
t_lex->context.first_name_resolution_table;> select_lex-
>context.table_list= > select_lex-
>context.first_name_resolution_table= second_table;> - res=
mysql_insert_select_prepare(thd);> - if (!res && (sel_result= new
(thd->mem_root) select_insert(thd,> + res=
mysql_insert_select_prepare(thd, lex->result ? lex->result : result);>
+ if (!res && (sel_result= new (thd->mem_root)
select_insert(with_returning_list, lex->result ? lex->result : result,
thd,> fir
st_table,>
first_table-
>table,>
&lex->field_list,> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy>
index 183a2504b70..3ef53350101 100644> --- a/sql/sql_yacc.yy> +++
b/sql/sql_yacc.yy> @@ -13333,10 +13338,15 @@
replace:> }> insert_field_spec> {>
- Lex->pop_select(); //main select> - if (Lex-
>check_main_unit_semantics())> - MYSQL_YYABORT;>
+ Lex->current_select->returning_list.swap(Lex-
>current_select->item_list);> }> + opt_select_ex
pressions> + {> + Lex->current_select-
>returning_list.swap(Lex->current_select->item_list);>
+ Lex->pop_select(); //main select> + if (Lex-
>check_main_unit_semantics())> + MYSQL_YYABORT;> +
}Please fix indentation.> ;> > insert_lock_option: