From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 5000C4696C3 for ; Tue, 14 Apr 2020 01:59:03 +0300 (MSK) Date: Mon, 13 Apr 2020 22:59:02 +0000 From: Nikita Pettik Message-ID: <20200413225901.GF24818@tarantool.org> References: <460ca5211700b8c47495879dfc8a75955eeb25b8.1586708735.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <460ca5211700b8c47495879dfc8a75955eeb25b8.1586708735.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] sql: do not change order of inserted values List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 12 Apr 19:29, imeevma@tarantool.org wrote: > @@ -365,18 +372,21 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) > } > } > > - for (uint32_t i = 0; i < field_count; ++i) { > + for (uint32_t i = 0; i < part_count; ++i) { > struct key_part_def *part = &ephemer_key_parts[i]; > - part->fieldno = i; I'd place comment explaining value of j and position of 'rowid'. > + uint32_t j = i; > + if (key_info != NULL && key_info->is_pk_rowid) > + j = field_count - 1; > + part->fieldno = j; > part->nullable_action = ON_CONFLICT_ACTION_NONE; > part->is_nullable = true; > part->sort_order = SORT_ORDER_ASC; > part->path = NULL; > - part->type = fields[i].type; > - part->coll_id = fields[i].coll_id; > + part->type = fields[j].type; > + part->coll_id = fields[j].coll_id; > } > struct key_def *ephemer_key_def = key_def_new(ephemer_key_parts, > - field_count, false); > + part_count, false); > if (ephemer_key_def == NULL) > return NULL; > > diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c > index 43a0de5..52a948d 100644 > --- a/src/box/sql/insert.c > +++ b/src/box/sql/insert.c > @@ -455,8 +455,23 @@ 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. > + * 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. > + */ So why don't you use this trick only if space with autoincrement is affected? So that proceed with old way for all other spaces. > + 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); > addrL = sqlVdbeAddOp1(v, OP_Yield, dest.iSDParm); > VdbeCoverage(v); > sqlVdbeAddOp2(v, OP_NextIdEphemeral, reg_eph, > diff --git a/test/sql/gh-4256-do-not-change-order-during-insertion.test.lua b/test/sql/gh-4256-do-not-change-order-during-insertion.test.lua > new file mode 100644 > index 0000000..4d8367c > --- /dev/null > +++ b/test/sql/gh-4256-do-not-change-order-during-insertion.test.lua > @@ -0,0 +1,15 @@ Btw, why don't you use SQL format of tests? > +test_run = require('test_run').new() > +engine = test_run:get_cfg('engine') > +-- > +-- Make sure that when inserting, values are inserted in the given > +-- order when ephemeral space is used. > +-- > +box.execute('CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT);') > +-- > +-- In order for this INSERT to use the ephemeral space, we created > +-- this trigger. > +-- > +box.execute('CREATE TRIGGER r AFTER INSERT ON t FOR EACH ROW BEGIN SELECT 1; END') > +box.execute('INSERT INTO t VALUES (1), (NULL), (10), (NULL), (NULL), (3), (NULL);') > +box.execute('SELECT * FROM t;') > +box.execute('DROP TABLE t;') > -- > 2.7.4 >