maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #11901
Re: INSERT ... RETURNING Initial code review
Hello!
I worked on the review and here is a report for the same.
CODE:
I fixed indentations and removed with_returning_list. Now I am using only
the select_result pointer to know whether returning_list is empty (except
in mysql_prepare_insert(). This function is using bool with_returning_list
which is passed to it as the last argument, instead of passing SELECT_LEX
structure)
I am not passing with_returning_list as argument anymore like previously so
I removed it from sql_parse.cc file in SQLCOM_INSERT_SELECT
Fixed the logic of my_ok() and send_eof(). I Tried sending my_ok()
regardless, but it was crashing the server. So I changed the previous code
to:
if(result)
result->send_eof()
else
::my_ok()
Now INSERT...IGNORE...RETURNING is working too.
Fixed the formatting in mysql_prepare_insert() (near setup_fileds() and
setup_wild())and made it more readable. For INSERT...SELECT, I am now
calling setup_fields() and setup_wild() only once-- in
select_insert::prepare(). I am passing the last argument = false in
mysql_prepare_insert() when it is called in mysql_insert_select_prepare(),
so that setup_field() and setup_wild() is only called once.
I moved returning_list to struct LEX so that current_select and
lex->first_select_lex() is not used.
Replaced the last argument in mysql_prepare_insert() (which was entire
SELECT_LEX structure) to bool with_returning_list. Now setup_field() and
setup_wild() is called only if with_returing_list is true.
Also restored the structure of SQL_I_List like it was before introducing
saved_first. I am now storing this table separately in class select_insert
as TABLE *insert_table
I also tried these queries. They are working fine.
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;
and
INSERT INTO t1 SELECT * FROM t1;
TESTS:
I removed redundant testcases, merged the others and fixed typo. Also
removed from all the test cases:
--disable_warnings
drop table if exists t1,t2;
--enable_warnings
#End of testcase
comments like table created successfully
Also I am now using echo only like this:
--echo #
--echo #SIMPLE INSERT STATEMENT
--echo #
Tested queries that return multiple rows and got error:
INSERT INTO t2(id2) VALUES (1) RETURNING (SELECT id1 from t1);
Error: subquery returns more than one row
I also tested queries like:
INSERT INTO t2(id2,val2) VALUES(3,'c') RETURNING (SELECT id1 FROM t2);
Error: t2 specified twice both as target for INSERT and as a separate
source for data
I have added these in test cases too.
INSERT INTO t2(id2,val2) VALUES(4,'d') RETURNING (SELECT * FROM t1);
This query fails specifically because there are multiple columns in the
subselect.
Also when there is only one column AND more than one row being returned:
INSERT INTO t2(id2,val2) VALUES(4,'d') RETURNING (SELECT id1 FROM t1);
Error: Subquery returns more than one row
But works fine when:
INSERT INTO t2(id2,val2) VALUES(4,'d') RETURNING (SELECT id1 FROM t1 where
id1=1);
Because it was returning only one row and column.
Regards,
Rucha Deodhar
On Sun, Jul 7, 2019 at 7:16 PM Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>
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:
>
>
References
