Hello! Thanks for the review. It will indeed help me write better code and test cases. To begin with, I' ll first get familiar with git bisect. I'll keep you updated about my progress. Regards, Rucha Deodhar On Sun, Jul 7, 2019, 7:16 PM Vicențiu Ciorbaru <vicentiu@mariadb.org> wrote:
Hi Rucha!
Here is the promised review. I have run the main test suite locally and have noticed
that some tests fail if you run with a debug build. The failure is an assertion
failure on the protocol level. This means you forgot to call a my_ok or my_error
at an appropriate time. I suggest you try and find which one of your commits
introduces this bug. A handy tool is `git bisect`. It's a great time to learn
about it, if you have not used it before.
There are quite a few other things to fix, but first, let's tackle the
initial review comments.
Now, on to the actual code.
Your text editor inserts tabs instead of spaces. Please configure it to only
insert 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_warnings
Test cases in MariaDB are written such that they do not leave behind any
tables or other elements from running the test. Thus, such statements are
not required (and actively discouraged) in new test cases. If tables from
previous tests are present and mysql-test-run (mtr) does not mark the test as
failed 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 means
the command was succesful. Also, when adding comments, I recommend you prefix
them 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 statement
Only 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 statement
Same 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 UPDATE
Same 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...SET
Same 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 copy
of the previous one and it has windows line endings. It's the only one that
has those. I suggest you move the statements not present in insert_parser.test
there 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_warnings
Similar 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 statement
See 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...SELECT
See 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 indent
the 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_warnings
See insert_returning.test
I 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 statement
See 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 case
too.
+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 statement
See 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 UPDATE
Typo 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 differ
Ok, 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_warnings
I 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 STATEMENT
See other test comments.
+INSERT INTO t1(num_int1,num_bit1,num_float1,num_double1,char_enum1,char_set1,str_varchar1, 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_varchar1, 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',dt1="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_varchar1, 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_varchar1, 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_varchar2, 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_varchar2, 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.result
How is this test different than previous ones? I am getting a bit confused
by 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 the
start. Do we need both with_returning_list and sel_result pointer? Can we
not use just the sel_result pointer and if it is null, we assume we do not
have 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);
+ }
}
else
{
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 have
changed. 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(thd, 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 within
mysql_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 impossible
to 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, both
in 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 case
for 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. Please
find 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 solution
first.
+
+ 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_result->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 resolution
without extending SQL_I_List.
select_lex->table_list.first= second_table;
+ select_lex->context.saved_table_list=select_lex->context.table_list;
+ select_lex->context.saved_name_resolution_table=
+ select_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,
first_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_expressions
+ {
+ 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: