[Tarantool-patches] [PATCH v2 2/2] sql: do not change order of inserted values
Nikita Pettik
korablev at tarantool.org
Tue Apr 14 01:59:02 MSK 2020
On 12 Apr 19:29, imeevma at 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
>
More information about the Tarantool-patches
mailing list