[Tarantool-patches] [PATCH v1 1/1] sql: use rowid as PK of ephemeral space on INSERT

Imeev Mergen imeevma at tarantool.org
Mon Mar 23 14:42:13 MSK 2020


On 3/23/20 2:40 PM, Mergen Imeev wrote:
> Hi! Thank you for the patch. My answers, diff and new
> version below.

Sorry, I mean "the review".


>
> On Wed, Mar 11, 2020 at 03:27:59PM +0000, Nikita Pettik wrote:
>> On 25 Feb 13:17, Mergen Imeev wrote:
>>> Hi! Thank you for review. My answers and new patch beliw.
>>> Date: Mon, 24 Feb 2020 16:44:07 +0300
>>> Subject: [PATCH] sql: do not change order of inserted values on INSERT
>>>
>>> Prior to this patch, if ephemeral space was used during INSERT,
>> Does fix affect only insert, but not replace? Or both?
> It affects both, but the problem appears only during INSERT
> since we cannot use REPLACE to insert NULL in field with
> AUTOINCREMENT. Changed the commit-message.
>
>>> the inserted values were sorted by the first column.
>> Are inserted values are always sorted during insertion op?
> No. It says "if ephemeral space was used", is this not
> enough?
>
>>> This can lead
>>> to an error when using the autoincrement feature.
>> Why? Could you please provide details? Not personally for me,
>> but in commit message.
> Extended the commit message.
>
>>> To avoid this,
>>> the patch makes it possible to make the rowid of the value
>>> inserted into the ephemeral space be the first part of
>>> ephemeral space index.
>> So, before patch rowid could be only last column of table?
> Now it is also the last column of the table.
>
>> And you achieve it extending sql_key_info with additional field?
> Yes.
>
>> All these tip-questions should help you to make commit message
>> more informative.
> Thanks.
>
>>> Closes #4256
>> Please attach a changelog to the patch.
> Done.
>
>>> diff --git a/src/box/sql.c b/src/box/sql.c
>>> index 1256df8..47cf235 100644
>>> --- a/src/box/sql.c
>>> +++ b/src/box/sql.c
>>> @@ -338,7 +338,10 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>>>   		return NULL;
>>>   	}
>>>   	for (uint32_t i = 0; i < field_count; ++i) {
>>> -		struct key_part_def *part = &ephemer_key_parts[i];
>>> +		uint32_t j = i;
>>> +		if (key_info != NULL && key_info->is_rowid_first)
>>> +			j = (j + 1) % field_count;
>> What is going on here??
> Cyclic shift.
>
>>> +		struct key_part_def *part = &ephemer_key_parts[j];
>>>   		part->fieldno = i;
>>>   		part->nullable_action = ON_CONFLICT_ACTION_NONE;
>>>   		part->is_nullable = true;
>>> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
>>> index 43a0de5..6ee728a 100644
>>> --- a/src/box/sql/insert.c
>>> +++ b/src/box/sql/insert.c
>>> @@ -455,8 +455,17 @@ 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.
>>> +			 */
>> Please, explain here why rowid should come as first column.
>> See reference example in sql_table_delete_from():
>>
>>     233			if (is_view) {
>>     234				/*
>>     235				 * At this stage SELECT is already materialized
>>     236				 * into ephemeral table, which has one additional
>>     237				 * tail field (except for ones specified in view
>>     238				 * format) which contains sequential ids. These ids
>>     239				 * are required since selected values may turn out to
>>     240				 * be non-unique. For instance:
>>     241				 * CREATE TABLE t (id INT PRIMARY KEY, a INT);
>>     242				 * INSERT INTO t VALUES (1, 1), (2, 1), (3, 1);
>>     243				 * CREATE VIEW v AS SELECT a FROM t;
>>
>> ...
>>
> Done.
>
>>> +			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_rowid_first = 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/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
>>> index d1fcf47..d9da921 100644
>>> --- a/src/box/sql/sqlInt.h
>>> +++ b/src/box/sql/sqlInt.h
>>> @@ -4111,6 +4111,8 @@ struct sql_key_info {
>>>   	struct key_def *key_def;
>>>   	/** Reference counter. */
>>>   	uint32_t refs;
>>> +	/** Rowid should be the first part of PK if true. */
>>> +	bool is_rowid_first;
>> Otherwise it is last or not presented at all?
> Yes. I changed the comment, but I am not sure if this is
> right. The only porpose of this flag is to show that if
> the rowid the first column. If this flag is false we may
> say that rowid may be not the first column.
>
>>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>>> index 620d74e..db782a7 100644
>>> --- a/src/box/sql/vdbe.c
>>> +++ b/src/box/sql/vdbe.c
>>> @@ -2702,8 +2702,25 @@ case OP_Column: {
>>>   		 * key parts.
>>>   		 */
>>>   		if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
>>> -			field_type = pC->uc.pCursor->index->def->
>>> -					key_def->parts[p2].type;
>>> +			struct key_def *key_def =
>>> +				pC->uc.pCursor->index->def->key_def;
>>> +			/*
>>> +			 * There are three options:
>>> +			 * |Rowid|First field|...|Last field|
>>> +			 * Or:
>>> +			 * |First field|...|Last field|
>>> +			 * Or:
>>> +			 * |First field|...|Last field|Rowid|
>>> +			 *
>>> +			 * If ephemeral space has a rowid column,
>> But it contradicts schema above.
> How?
>
>>> +			 * it is always the last column. Due to
>>> +			 * this, the field number of the rowid
>>> +			 * column cannot be 0.
>>> +			 */
>>> +			int partno = p2;
>>> +			if (key_def->parts[0].fieldno != 0)
>>> +				partno = (partno + 1) % key_def->part_count;
>> See this for the second time, still don't understand what it is
>> supposed to mean.
> Simplified a bit and extended the comment.
>
>>> +			field_type = key_def->parts[partno].type;
>>>   		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>>>   			field_type = pC->uc.pCursor->space->def->
>>>   					fields[p2].type;
>>> diff --git a/test/sql/autoincrement.test.lua b/test/sql/autoincrement.test.lua
>>> index 63a902a..99f4813 100644
>>> --- a/test/sql/autoincrement.test.lua
>>> +++ b/test/sql/autoincrement.test.lua
>>> @@ -31,3 +31,13 @@ seqs = box.space._sequence:select{}
>>>   #seqs == 0 or seqs
>>>   
>>>   box.schema.user.drop('user1')
>>> +
>>> +--
>>> +-- gh-4256: 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);')
>>> +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);')
>> Leave explanation why no-op trigger is required. Otherwise it
>> looks redundant.
> Done.
>
>> Also, Kirill forces new convention saying that
>> each bug fix should be placed at separate file named gh-xxxx-description.
> Done.
>
>>> +box.execute('SELECT * FROM t;')
>>> +box.execute('DROP TABLE t;')
>
> Diff:
>
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 47cf235..8c25095 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -339,6 +339,12 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>   	}
>   	for (uint32_t i = 0; i < field_count; ++i) {
>   		uint32_t j = i;
> +		/*
> +		 * In case we know that the last column is rowid,
> +		 * and we want to make it the first part of the
> +		 * index, we will do a cyclic shift. Thus, we will
> +		 * not disrupt the order of other columns.
> +		 */
>   		if (key_info != NULL && key_info->is_rowid_first)
>   			j = (j + 1) % field_count;
>   		struct key_part_def *part = &ephemer_key_parts[j];
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 6ee728a..28e3096 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -458,6 +458,12 @@ sqlInsert(Parse * pParse,	/* Parser context */
>   			/*
>   			 * 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.
>   			 */
>   			struct sql_key_info *key_info =
>   				sql_key_info_new(pParse->db, nColumn + 1);
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 9c00a7b..a5f0249 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -4114,7 +4114,11 @@ struct sql_key_info {
>   	struct key_def *key_def;
>   	/** Reference counter. */
>   	uint32_t refs;
> -	/** Rowid should be the first part of PK if true. */
> +	/**
> +	 * Rowid should be the first part of PK, if true. If this
> +	 * flag is false, rowid may be any part of the index or
> +	 * may be absent.
> +	 */
>   	bool is_rowid_first;
>   	/** Number of parts in the key. */
>   	uint32_t part_count;
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index a200063..e5cfa5e 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2715,11 +2715,21 @@ case OP_Column: {
>   			 * If ephemeral space has a rowid column,
>   			 * it is always the last column. Due to
>   			 * this, the field number of the rowid
> -			 * column cannot be 0.
> +			 * column cannot be 0. So, if the first
> +			 * part of the index have something other
> +			 * than 0 as a fieldno, we know that this
> +			 * is fieldno of rowid, since in all other
> +			 * cases fieldno of the first part is 0.
> +			 *
> +			 * If rowid is the first part of the
> +			 * index, we know that all the other parts
> +			 * are columns sorted in the required
> +			 * order. So, we should increment partno
> +			 * by 1.
>   			 */
> -			int partno = p2;
> +			uint32_t partno = p2;
>   			if (key_def->parts[0].fieldno != 0)
> -				partno = (partno + 1) % key_def->part_count;
> +				++partno;
>   			field_type = key_def->parts[partno].type;
>   		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>   			field_type = pC->uc.pCursor->space->def->
> diff --git a/test/sql/autoincrement.result b/test/sql/autoincrement.result
> index 7d3e902..ca3d6f3 100644
> --- a/test/sql/autoincrement.result
> +++ b/test/sql/autoincrement.result
> @@ -78,43 +78,3 @@ seqs = box.space._sequence:select{}
>   box.schema.user.drop('user1')
>    | ---
>    | ...
> -
> ---
> --- gh-4256: 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);')
> - | ---
> - | - row_count: 1
> - | ...
> -box.execute('CREATE TRIGGER r AFTER INSERT ON t FOR EACH ROW BEGIN SELECT 1; END')
> - | ---
> - | - row_count: 1
> - | ...
> -box.execute('INSERT INTO t VALUES (1), (NULL), (10), (NULL), (NULL), (3), (NULL);')
> - | ---
> - | - autoincrement_ids:
> - |   - 2
> - |   - 11
> - |   - 12
> - |   - 13
> - |   row_count: 7
> - | ...
> -box.execute('SELECT * FROM t;')
> - | ---
> - | - metadata:
> - |   - name: I
> - |     type: integer
> - |   rows:
> - |   - [1]
> - |   - [2]
> - |   - [3]
> - |   - [10]
> - |   - [11]
> - |   - [12]
> - |   - [13]
> - | ...
> -box.execute('DROP TABLE t;')
> - | ---
> - | - row_count: 1
> - | ...
> diff --git a/test/sql/autoincrement.test.lua b/test/sql/autoincrement.test.lua
> index 99f4813..63a902a 100644
> --- a/test/sql/autoincrement.test.lua
> +++ b/test/sql/autoincrement.test.lua
> @@ -31,13 +31,3 @@ seqs = box.space._sequence:select{}
>   #seqs == 0 or seqs
>   
>   box.schema.user.drop('user1')
> -
> ---
> --- gh-4256: 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);')
> -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;')
> diff --git a/test/sql/gh-4256-do-not-change-order-during-insertion.result b/test/sql/gh-4256-do-not-change-order-during-insertion.result
> new file mode 100644
> index 0000000..56e28b5
> --- /dev/null
> +++ b/test/sql/gh-4256-do-not-change-order-during-insertion.result
> @@ -0,0 +1,50 @@
> +-- test-run result file version 2
> +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);')
> + | ---
> + | - row_count: 1
> + | ...
> +--
> +-- 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')
> + | ---
> + | - row_count: 1
> + | ...
> +box.execute('INSERT INTO t VALUES (1), (NULL), (10), (NULL), (NULL), (3), (NULL);')
> + | ---
> + | - autoincrement_ids:
> + |   - 2
> + |   - 11
> + |   - 12
> + |   - 13
> + |   row_count: 7
> + | ...
> +box.execute('SELECT * FROM t;')
> + | ---
> + | - metadata:
> + |   - name: I
> + |     type: integer
> + |   rows:
> + |   - [1]
> + |   - [2]
> + |   - [3]
> + |   - [10]
> + |   - [11]
> + |   - [12]
> + |   - [13]
> + | ...
> +box.execute('DROP TABLE t;')
> + | ---
> + | - row_count: 1
> + | ...
> 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 @@
> +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;')
>
>
> New version:
>
>
>  From 4244c170e0b22fa1084d8d9e1fac9c252b9b73cd Mon Sep 17 00:00:00 2001
> Message-Id: <4244c170e0b22fa1084d8d9e1fac9c252b9b73cd.1584962777.git.imeevma at gmail.com>
> From: Mergen Imeev <imeevma at gmail.com>
> Date: Mon, 24 Feb 2020 16:44:07 +0300
> Subject: [PATCH v2 1/1] sql: do not change order of inserted values
>
> Before this patch, if an ephemeral space was used during INSERT or
> REPLACE, the inserted values were sorted by the first column,
> since this was the first part of the index. This can lead to an
> error when using the AUTOINCREMENT feature, since changing the
> order of the inserted value can change the value inserted instead
> of NULL. To avoid this, the patch makes the rowid of the inserted
> row in the ephemeral space the first part of the ephemeral space
> index. Currently, if the ephemeral space has a rowid, it is always
> the last column of the ephemeral space. So, to make rowid the
> first part of the index, a cyclic shift is used. Thus, we will not
> change the order of all other columns.
>
> Closes #4256
> ---
> https://github.com/tarantool/tarantool/issues/4256
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4256-fix-order-during-insertion
>
> @ChangeLog
>   - During INSERT and REPLACE the order of inserted rows will not
>     be changed (gh-4256).
>
>   src/box/sql.c                                      | 11 ++++-
>   src/box/sql/insert.c                               | 19 +++++++-
>   src/box/sql/select.c                               |  2 +
>   src/box/sql/sqlInt.h                               |  6 +++
>   src/box/sql/vdbe.c                                 | 31 +++++++++++++-
>   ...256-do-not-change-order-during-insertion.result | 50 ++++++++++++++++++++++
>   ...6-do-not-change-order-during-insertion.test.lua | 15 +++++++
>   7 files changed, 129 insertions(+), 5 deletions(-)
>   create mode 100644 test/sql/gh-4256-do-not-change-order-during-insertion.result
>   create mode 100644 test/sql/gh-4256-do-not-change-order-during-insertion.test.lua
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index 1256df8..8c25095 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -338,7 +338,16 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>   		return NULL;
>   	}
>   	for (uint32_t i = 0; i < field_count; ++i) {
> -		struct key_part_def *part = &ephemer_key_parts[i];
> +		uint32_t j = i;
> +		/*
> +		 * In case we know that the last column is rowid,
> +		 * and we want to make it the first part of the
> +		 * index, we will do a cyclic shift. Thus, we will
> +		 * not disrupt the order of other columns.
> +		 */
> +		if (key_info != NULL && key_info->is_rowid_first)
> +			j = (j + 1) % field_count;
> +		struct key_part_def *part = &ephemer_key_parts[j];
>   		part->fieldno = i;
>   		part->nullable_action = ON_CONFLICT_ACTION_NONE;
>   		part->is_nullable = true;
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 43a0de5..28e3096 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.
> +			 */
> +			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_rowid_first = 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/src/box/sql/select.c b/src/box/sql/select.c
> index 65e41f2..105a4a3 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -1422,6 +1422,7 @@ sql_key_info_new(sql *db, uint32_t part_count)
>   	key_info->key_def = NULL;
>   	key_info->refs = 1;
>   	key_info->part_count = part_count;
> +	key_info->is_rowid_first = false;
>   	for (uint32_t i = 0; i < part_count; i++) {
>   		struct key_part_def *part = &key_info->parts[i];
>   		part->fieldno = i;
> @@ -1448,6 +1449,7 @@ sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def)
>   	key_info->key_def = NULL;
>   	key_info->refs = 1;
>   	key_info->part_count = key_def->part_count;
> +	key_info->is_rowid_first = false;
>   	key_def_dump_parts(key_def, key_info->parts, NULL);
>   	return key_info;
>   }
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 1579cc9..a5f0249 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -4114,6 +4114,12 @@ struct sql_key_info {
>   	struct key_def *key_def;
>   	/** Reference counter. */
>   	uint32_t refs;
> +	/**
> +	 * Rowid should be the first part of PK, if true. If this
> +	 * flag is false, rowid may be any part of the index or
> +	 * may be absent.
> +	 */
> +	bool is_rowid_first;
>   	/** Number of parts in the key. */
>   	uint32_t part_count;
>   	/** Definition of the key parts. */
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index e8a029a..e5cfa5e 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2702,8 +2702,35 @@ case OP_Column: {
>   		 * key parts.
>   		 */
>   		if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
> -			field_type = pC->uc.pCursor->index->def->
> -					key_def->parts[p2].type;
> +			struct key_def *key_def =
> +				pC->uc.pCursor->index->def->key_def;
> +			/*
> +			 * There are three options:
> +			 * |Rowid|First field|...|Last field|
> +			 * Or:
> +			 * |First field|...|Last field|
> +			 * Or:
> +			 * |First field|...|Last field|Rowid|
> +			 *
> +			 * If ephemeral space has a rowid column,
> +			 * it is always the last column. Due to
> +			 * this, the field number of the rowid
> +			 * column cannot be 0. So, if the first
> +			 * part of the index have something other
> +			 * than 0 as a fieldno, we know that this
> +			 * is fieldno of rowid, since in all other
> +			 * cases fieldno of the first part is 0.
> +			 *
> +			 * If rowid is the first part of the
> +			 * index, we know that all the other parts
> +			 * are columns sorted in the required
> +			 * order. So, we should increment partno
> +			 * by 1.
> +			 */
> +			uint32_t partno = p2;
> +			if (key_def->parts[0].fieldno != 0)
> +				++partno;
> +			field_type = key_def->parts[partno].type;
>   		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>   			field_type = pC->uc.pCursor->space->def->
>   					fields[p2].type;
> diff --git a/test/sql/gh-4256-do-not-change-order-during-insertion.result b/test/sql/gh-4256-do-not-change-order-during-insertion.result
> new file mode 100644
> index 0000000..56e28b5
> --- /dev/null
> +++ b/test/sql/gh-4256-do-not-change-order-during-insertion.result
> @@ -0,0 +1,50 @@
> +-- test-run result file version 2
> +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);')
> + | ---
> + | - row_count: 1
> + | ...
> +--
> +-- 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')
> + | ---
> + | - row_count: 1
> + | ...
> +box.execute('INSERT INTO t VALUES (1), (NULL), (10), (NULL), (NULL), (3), (NULL);')
> + | ---
> + | - autoincrement_ids:
> + |   - 2
> + |   - 11
> + |   - 12
> + |   - 13
> + |   row_count: 7
> + | ...
> +box.execute('SELECT * FROM t;')
> + | ---
> + | - metadata:
> + |   - name: I
> + |     type: integer
> + |   rows:
> + |   - [1]
> + |   - [2]
> + |   - [3]
> + |   - [10]
> + |   - [11]
> + |   - [12]
> + |   - [13]
> + | ...
> +box.execute('DROP TABLE t;')
> + | ---
> + | - row_count: 1
> + | ...
> 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 @@
> +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;')


More information about the Tarantool-patches mailing list