Re: [Tarantool-patches] [PATCH v4 6/6] sql: support column addition

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

Thanks for the patch!

See 3 comments below.

On 11.09.2020 23:51, Roman Khabibov wrote:
> Enable to add column to existing space with
> <ALTER TABLE ADD [COLUMN]> statement. Column definition can be
> supplemented with the four types of constraints, <DEFAULT>,
> <COLLATE> clauses and <[NOT] NULL>, AUTOINCREMENT.
> 
> Closes #2349, #3075
> 
> @TarantoolBot document
> Title: Add columns to existing tables in SQL
> Now, it is possible to add columns to existing empty spaces using
> <ALTER TABLE table_name ADD [COLUMN] column_name column_type ...>
> statement. The column definition is the same as in <CREATE TABLE>
> statement.
> 
> For example:
> 
> ```
> tarantool> box.execute("CREATE TABLE test (a INTEGER PRIMARY KEY)")
> ---
> - row_count: 1
> ...
> 
> tarantool> box.execute([[ALTER TABLE test ADD COLUMN b TEXT
>          >                                           CHECK (LENGTH(b) > 1)
>          >                                           NOT NULL
>          >                                           DEFAULT ('aa')
>          >                                           COLLATE "unicode_ci"
>          >             ]])
> ---
> - row_count: 0

1. It is row_count 1, not 0.

> ...
> ```
> ---
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 3c21375f5..ec16399a0 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -271,6 +271,7 @@ struct errcode_record {
>          /*216 */_(ER_SYNC_QUORUM_TIMEOUT,       "Quorum collection for a synchronous transaction is timed out") \
>          /*217 */_(ER_SYNC_ROLLBACK,             "A rollback for a synchronous transaction is received") \
>  	/*218 */_(ER_TUPLE_METADATA_IS_TOO_BIG,	"Can't create tuple: metadata size %u is too big") \
> +	/*219 */_(ER_SQL_CANT_ADD_AUTOINC,	"Can't add AUTOINCREMENT: the space '%s' already has one auto incremented field") \

2. Probably it can be used inside sql_add_autoincrement() too. To
make the same error have the same message.

>  /*
>   * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index d1d240315..677099b48 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1032,18 +1180,21 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context,
>  			  P4_DYNAMIC);
>  	/*
>  	 * In case we are adding FK constraints during execution
> -	 * of <CREATE TABLE ...> statement, we don't have child
> -	 * id, but we know register where it will be stored.
> +	 * of <CREATE TABLE ...> or <ALER TABLE ADD COLUMN ...>

3. ALER -> ALTER.