[tarantool-patches] Re: [PATCH v2 1/4] sql: don't increment row count on FK creation within CREATE TABLE

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the patch!

See 1 comment below, my fixes at the end of the email
and on the branch.

On 13/11/2018 19:11, Nikita Pettik wrote:
> We have agreement that each successful DDL operation returns 1 (one) as
> a row count (via IProto protocol or changes() SQL function), despite
> the number of other created objects (e.g. indexes, sequences, FK
> constraints etc).
> 
> Needed for #2181
> ---
>   src/box/sql/build.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 5b3348bd2..e28856e26 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1415,7 +1415,8 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk)
>   			  constr_tuple_reg + 9);
>   	sqlite3VdbeAddOp3(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, 0,
>   			  constr_tuple_reg + 9);
> -	sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE);
> +	if (parse_context->pNewTable == NULL)
> +		sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE);
>   	save_record(parse_context, BOX_FK_CONSTRAINT_ID, constr_tuple_reg, 2,
>   		    vdbe->nOp - 1);
>   	sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10);
> 

I added a test to this, and noticed, that 'drop table' returns
rowcount 2 when it has a foreign key. Please, fix. Now my test
fails on it.

==============================================================

commit bfdc94f2ab1cf29f7ae1948a3fa002855a6cf430
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Wed Nov 14 15:10:02 2018 +0300

     Review fixes [tests fail]

diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index f390a73a9..e75f79110 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -727,9 +727,6 @@ cn:execute('insert into TEST3 values (null), (null), (null), (null)')
    - -29
    rowcount: 4
  ...
-cn:close()
----
-...
  box.sql.execute('drop table test')
  ---
  ...
@@ -742,6 +739,28 @@ sq:drop()
  box.sql.execute('drop table test3')
  ---
  ...
+--
+-- Ensure that FK inside CREATE TABLE does not affect rowcount.
+--
+cn:execute('create table test (id integer primary key)')
+---
+- rowcount: 1
+...
+cn:execute('create table test2 (id integer primary key, ref integer references test)')
+---
+- rowcount: 1
+...
+cn:execute('drop table test2')
+---
+- rowcount: 1
+...
+cn:execute('drop table test')
+---
+- rowcount: 1
+...
+cn:close()
+---
+...
  box.schema.user.revoke('guest', 'read,write,execute', 'universe')
  ---
  ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index a18755b00..fb5685a9e 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -225,12 +225,21 @@ box.sql.execute('create table test3 (id int primary key autoincrement)')
  box.schema.sequence.alter('TEST3', {min=-10000, step=-10})
  cn:execute('insert into TEST3 values (null), (null), (null), (null)')
  
-cn:close()
  box.sql.execute('drop table test')
  s:drop()
  sq:drop()
  box.sql.execute('drop table test3')
  
+--
+-- Ensure that FK inside CREATE TABLE does not affect rowcount.
+--
+cn:execute('create table test (id integer primary key)')
+cn:execute('create table test2 (id integer primary key, ref integer references test)')
+cn:execute('drop table test2')
+cn:execute('drop table test')
+
+cn:close()
+
  box.schema.user.revoke('guest', 'read,write,execute', 'universe')
  box.schema.user.revoke('guest', 'create', 'space')
  space = nil