From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37])
 (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 C236A4696C3
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 16 Apr 2020 03:09:58 +0300 (MSK)
Date: Thu, 16 Apr 2020 00:09:57 +0000
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200416000957.GB2581@tarantool.org>
References: <cover.1586933931.git.imeevma@gmail.com>
 <2542c80348de25f26cac73e101b394a1873156da.1586933931.git.imeevma@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <2542c80348de25f26cac73e101b394a1873156da.1586933931.git.imeevma@gmail.com>
Subject: Re: [Tarantool-patches] [PATCH v3 3/3] sql: do not change order of
	inserted values
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: imeevma@tarantool.org
Cc: tarantool-patches@dev.tarantool.org

On 15 Apr 10:09, imeevma@tarantool.org wrote:
> My answers and new patch below.
> 
> On 4/14/20 1:59 AM, Nikita Pettik wrote:
>  
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 43a0de5..787855e 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -455,8 +455,31 @@ sqlInsert(Parse * pParse,	/* Parser context */
>  			reg_eph = ++pParse->nMem;
>  			regRec = sqlGetTempReg(pParse);
>  			regCopy = sqlGetTempRange(pParse, nColumn + 1);
> -			sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
> -					  nColumn + 1);
> +			/*
> +			 * This key_info is used to show that
> +			 * rowid should be the first part of PK in
> +			 * case we used AUTOINCREMENT feature.
> +			 * This way we will save initial order of
> +			 * the inserted values. The order is
> +			 * important if we use the AUTOINCREMENT
> +			 * feature, since changing the order can
> +			 * change the number inserted instead of
> +			 * NULL.
> +			 */
> +			if (space->sequence != NULL) {
> +				struct sql_key_info *key_info =
> +					sql_key_info_new(pParse->db,
> +							 nColumn + 1);
> +				key_info->parts[nColumn].type =
> +					FIELD_TYPE_UNSIGNED;
> +				key_info->is_pk_rowid = true;
> +				sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
> +					      nColumn + 1, 0, (char *)key_info,
> +					      P4_KEYINFO);
> +			} else {
> +				sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
> +					      nColumn + 1);
> +			}

You can remove 'else' branch and always add OP_OPenTEphemeral with
2 operands. Then inside 'if' branch invoke sqlVdbeChangeP4().
Otherwise LGTM.