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

[n.pettik]

> 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.

Thanks for provided test. Fixed:

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index e28856e26..9c9a6d9e1 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1810,7 +1810,6 @@ vdbe_emit_fkey_drop(struct Parse *parse_context, char *constraint_name,
        }
        sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, key_reg, 2, key_reg + 2);
        sqlite3VdbeAddOp2(vdbe, OP_SDelete, BOX_FK_CONSTRAINT_ID, key_reg + 2);
-       sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE);
        VdbeComment((vdbe, "Delete FK constraint %s", constraint_name));
        sqlite3ReleaseTempRange(parse_context, key_reg, 3);
 }
@@ -2269,6 +2268,13 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table,
        if (constraint_name != NULL)
                vdbe_emit_fkey_drop(parse_context, constraint_name,
                                    child->def->id);
+       /*
+        * We account changes to row count only if drop of
+        * foreign keys take place in a separate
+        * ALTER TABLE DROP CONSTRAINT statement, since whole
+        * DROP TABLE always returns 1 (one) as a row count.
+        */
+       sqlite3VdbeChangeP5(sqlite3GetVdbe(parse_context), OPFLAG_NCHANGE);