[tarantool-patches] Re: [PATCH v2 5/5] sql: introduce ALTER TABLE ADD CONSTRAINT UNIQUE/PRIMARY KEY

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

Thanks for the patch! See 1 comment below.

On 23/01/2019 20:56, Nikita Pettik wrote:
> Table (aka space) can be created without indexes at least from Lua-land
> (note that according ANSI SQL table may lack PK). Since there were no
> facilities to create primary key constraint on already existing table,
> lets extend ADD CONSTRAINT statement with UNIQUE and PRIMARY KEY
> clauses. In this case, UNIQUE works exactly in the same way as CREATE
> UNIQUE INDEX ... statement does.  In Tarantool primary index is an index
> with id == 0, so during execution of ADD CONSTRAINT PRIMARY KEY we check
> that there is no any entries in _index space and create that one.
> Otherwise, error is raised.
> 
> Part of #3097
> Follow-up #3914
> ---
>   src/box/sql/build.c          | 29 ++++++++++++++++++++--
>   src/box/sql/parse.y          | 20 ++++++++++++++++
>   test/sql-tap/alter.test.lua  | 57 +++++++++++++++++++++++++++++++++++++++++++-
>   test/sql-tap/index1.test.lua | 11 ++++++++-
>   4 files changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 660d5acfc..6eb9718be 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2103,6 +2103,19 @@ generate_index_id(struct Parse *parse, uint32_t space_id, int cursor)
>   	return iid_reg;
>   }
>   
> +static void
> +pk_check_existence(struct Parse *parse, uint32_t space_id, int _index_cursor)

1. Maybe Kostja is right. Could you rename it please to vdbe_emit_pk_existence_check?
I see, that it does not use parser for anything but to just get Vdbe, so I think
we can just pass Vdbe.

I do not think, that such a simple function is worth a comment, but if Kostja
asked, then lets add a one.

> +{
> +	struct Vdbe *v = sqlite3GetVdbe(parse);
> +	int tmp_reg = ++parse->nMem;
> +	sqlite3VdbeAddOp2(v, OP_Integer, space_id, tmp_reg);
> +	int found_addr = sqlite3VdbeAddOp4Int(v, OP_NotFound, _index_cursor, 0,
> +					     tmp_reg, 1);
> +	sqlite3VdbeAddOp4(v, OP_Halt, SQLITE_ERROR, ON_CONFLICT_ACTION_FAIL, 0,
> +			  "multiple primary keys are not allowed", P4_STATIC);
> +	sqlite3VdbeJumpHere(v, found_addr);
> +}