From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id B5284469719
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 17 Sep 2020 18:46:01 +0300 (MSK)
References: <20200911215115.6622-1-roman.habibov@tarantool.org>
 <20200911215115.6622-7-roman.habibov@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <791ca74b-ec48-1fac-e674-1c5d78ae100d@tarantool.org>
Date: Thu, 17 Sep 2020 17:45:59 +0200
MIME-Version: 1.0
In-Reply-To: <20200911215115.6622-7-roman.habibov@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v4 6/6] sql: support column addition
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Roman Khabibov <roman.habibov@tarantool.org>, tarantool-patches@dev.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.