[Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[imeevma]

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 only part of the ephemeral space
index.

Closes #4256
---
https://github.com/tarantool/tarantool/tree/imeevma/gh-4256-fix-order-during-insertion
https://github.com/tarantool/tarantool/issues/4256

#ChangeLog
 - The inserted values will now always be inserted in the order in
   which they were given (gh-4256).

 src/box/sql.c                                      | 26 +++++++----
 src/box/sql/insert.c                               | 19 +++++++-
 src/box/sql/select.c                               |  2 +
 src/box/sql/sqlInt.h                               |  2 +
 src/box/sql/vdbe.c                                 | 19 ++------
 ...256-do-not-change-order-during-insertion.result | 50 ++++++++++++++++++++++
 ...6-do-not-change-order-during-insertion.test.lua | 15 +++++++
 7 files changed, 107 insertions(+), 26 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 27089bd..7c1eba2 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -324,10 +324,17 @@ struct space *
 sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
 {
 	struct key_def *def = NULL;
+	uint32_t part_count = field_count;
 	if (key_info != NULL) {
 		def = sql_key_info_to_key_def(key_info);
 		if (def == NULL)
 			return NULL;
+		/*
+		 * In case is_pk_rowid is true we can use rowid
+		 * as the only part of the key.
+		 */
+		if (key_info->is_pk_rowid)
+			part_count = 1;
 	}
 
 	struct region *region = &fiber()->gc;
@@ -339,12 +346,12 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
 	 */
 	assert(SQL_MAX_COLUMN < 9999);
 	uint32_t name_len = sizeof("_COLUMN_") + 5;
-	uint32_t size = field_count * (sizeof(struct field_def) + name_len +
-				       sizeof(struct key_part_def));
+	uint32_t size = field_count * (sizeof(struct field_def) + name_len) +
+			part_count * sizeof(struct key_part_def);
 	struct field_def *fields = region_alloc(region, size);
 	struct key_part_def *parts = (void *)fields +
 				     field_count * sizeof(struct field_def);
-	char *names = (char *)parts + field_count * sizeof(struct key_part_def);
+	char *names = (char *)parts + part_count * sizeof(struct key_part_def);
 	for (uint32_t i = 0; i < field_count; ++i) {
 		struct field_def *field = &fields[i];
 		field->name = names;
@@ -364,17 +371,20 @@ 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 = &parts[i];
-		part->fieldno = i;
+		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 *key_def = key_def_new(parts, field_count, false);
+	struct key_def *key_def = key_def_new(parts, part_count, false);
 	if (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.
+			 */
+			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/src/box/sql/select.c b/src/box/sql/select.c
index 65e41f2..f394840 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_pk_rowid = 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_pk_rowid = 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..261c56e 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4114,6 +4114,8 @@ struct sql_key_info {
 	struct key_def *key_def;
 	/** Reference counter. */
 	uint32_t refs;
+	/** Rowid should be the only part of PK, if true. */
+	bool is_pk_rowid;
 	/** 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..56c9d82 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2694,23 +2694,10 @@ case OP_Column: {
 		pC->cacheStatus = p->cacheCtr;
 	}
 	enum field_type field_type = field_type_MAX;
-	if (pC->eCurType == CURTYPE_TARANTOOL) {
-		/*
-		 * Ephemeral spaces feature only one index
-		 * covering all fields, but ephemeral spaces
-		 * lack format. So, we can fetch type from
-		 * key parts.
-		 */
-		if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
-			field_type = pC->uc.pCursor->index->def->
-					key_def->parts[p2].type;
-		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
-			field_type = pC->uc.pCursor->space->def->
-					fields[p2].type;
-		}
-	} else if (pC->eCurType == CURTYPE_SORTER) {
+	if (pC->eCurType == CURTYPE_TARANTOOL)
+		field_type = pC->uc.pCursor->space->def->fields[p2].type;
+	else if (pC->eCurType == CURTYPE_SORTER)
 		field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
-	}
 	struct Mem *default_val_mem =
 		pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
 	if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)
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;')
-- 
2.7.4

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

See 2 questions below.

> diff --git a/src/box/sql.c b/src/box/sql.c
> index 27089bd..7c1eba2 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -324,10 +324,17 @@ struct space *
>  sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>  {
>  	struct key_def *def = NULL;
> +	uint32_t part_count = field_count;
>  	if (key_info != NULL) {
>  		def = sql_key_info_to_key_def(key_info);
>  		if (def == NULL)
>  			return NULL;
> +		/*
> +		 * In case is_pk_rowid is true we can use rowid
> +		 * as the only part of the key.
> +		 */
> +		if (key_info->is_pk_rowid)
> +			part_count = 1;

1. On one hand this can really speed up ephemeral space comparators,
because we have special templates for single indexed unsigned field
if it is in 1, 2, or 3 column.

On the other hand this won't help, because rowid is the last column
somewhy. And these fast comparators won't help.

Can we make rowid first column?

>  	}
>  
>  	struct region *region = &fiber()->gc;
> @@ -364,17 +371,20 @@ 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 = &parts[i];
> -		part->fieldno = i;
> +		uint32_t j = i;
> +		if (key_info != NULL && key_info->is_pk_rowid)
> +			j = field_count - 1;

2. Does it make sense to use part count > 1, if we have rowid?
It is unique anyway.

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Mergen Imeev]

Hi! Thank you for review. My answers below.

On Mon, Apr 06, 2020 at 11:42:39PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 2 questions below.
> 
> > diff --git a/src/box/sql.c b/src/box/sql.c
> > index 27089bd..7c1eba2 100644
> > --- a/src/box/sql.c
> > +++ b/src/box/sql.c
> > @@ -324,10 +324,17 @@ struct space *
> >  sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> >  {
> >  	struct key_def *def = NULL;
> > +	uint32_t part_count = field_count;
> >  	if (key_info != NULL) {
> >  		def = sql_key_info_to_key_def(key_info);
> >  		if (def == NULL)
> >  			return NULL;
> > +		/*
> > +		 * In case is_pk_rowid is true we can use rowid
> > +		 * as the only part of the key.
> > +		 */
> > +		if (key_info->is_pk_rowid)
> > +			part_count = 1;
> 
> 1. On one hand this can really speed up ephemeral space comparators,
> because we have special templates for single indexed unsigned field
> if it is in 1, 2, or 3 column.
> 
> On the other hand this won't help, because rowid is the last column
> somewhy. And these fast comparators won't help.
> 
> Can we make rowid first column?
> 
I think we can, but that sould be quite confusing since not
all ephemeral spaces have rowid. I am not sure that we
really need this.

> >  	}
> >  
> >  	struct region *region = &fiber()->gc;
> > @@ -364,17 +371,20 @@ 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 = &parts[i];
> > -		part->fieldno = i;
> > +		uint32_t j = i;
> > +		if (key_info != NULL && key_info->is_pk_rowid)
> > +			j = field_count - 1;
> 
> 2. Does it make sense to use part count > 1, if we have rowid?
> It is unique anyway.
True, but not all ephemeral spaces have rowid, so we cannot
do this all the time. Still, we can do this in all cases an
ephemeral space have rowid. This may affect performance.
Should I do it in this patch? Or may I just create an issue
on github?

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Vladislav Shpilevoy]

Hi! Thanks for the answers!

> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2694,23 +2694,10 @@ case OP_Column: {
>  		pC->cacheStatus = p->cacheCtr;
>  	}
>  	enum field_type field_type = field_type_MAX;
> -	if (pC->eCurType == CURTYPE_TARANTOOL) {
> -		/*
> -		 * Ephemeral spaces feature only one index
> -		 * covering all fields, but ephemeral spaces
> -		 * lack format. So, we can fetch type from
> -		 * key parts.
> -		 */
> -		if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
> -			field_type = pC->uc.pCursor->index->def->
> -					key_def->parts[p2].type;
> -		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
> -			field_type = pC->uc.pCursor->space->def->
> -					fields[p2].type;
> -		}
> -	} else if (pC->eCurType == CURTYPE_SORTER) {
> +	if (pC->eCurType == CURTYPE_TARANTOOL)
> +		field_type = pC->uc.pCursor->space->def->fields[p2].type;
> +	else if (pC->eCurType == CURTYPE_SORTER)
>  		field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
> -	}
>  	struct Mem *default_val_mem =
>  		pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
>  	if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)

Shouldn't this be a part of the previous commit, which
introduces the ephemeral space format?

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Imeev Mergen]

Hi! Thanks for review. My answer below.

On 4/11/20 5:34 PM, Vladislav Shpilevoy wrote:
> Hi! Thanks for the answers!
>
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -2694,23 +2694,10 @@ case OP_Column: {
>>   		pC->cacheStatus = p->cacheCtr;
>>   	}
>>   	enum field_type field_type = field_type_MAX;
>> -	if (pC->eCurType == CURTYPE_TARANTOOL) {
>> -		/*
>> -		 * Ephemeral spaces feature only one index
>> -		 * covering all fields, but ephemeral spaces
>> -		 * lack format. So, we can fetch type from
>> -		 * key parts.
>> -		 */
>> -		if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
>> -			field_type = pC->uc.pCursor->index->def->
>> -					key_def->parts[p2].type;
>> -		} else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>> -			field_type = pC->uc.pCursor->space->def->
>> -					fields[p2].type;
>> -		}
>> -	} else if (pC->eCurType == CURTYPE_SORTER) {
>> +	if (pC->eCurType == CURTYPE_TARANTOOL)
>> +		field_type = pC->uc.pCursor->space->def->fields[p2].type;
>> +	else if (pC->eCurType == CURTYPE_SORTER)
>>   		field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
>> -	}
>>   	struct Mem *default_val_mem =
>>   		pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
>>   	if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)
> Shouldn't this be a part of the previous commit, which
> introduces the ephemeral space format?
They should, along with 3962. But, they have different milistones,
I wasn't sure that it is right to put all three tickets in one
patch-set.

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Vladislav Shpilevoy]

>>> --- a/src/box/sql/vdbe.c
>>> +++ b/src/box/sql/vdbe.c
>>> @@ -2694,23 +2694,10 @@ case OP_Column: {
>>>           pC->cacheStatus = p->cacheCtr;
>>>       }
>>>       enum field_type field_type = field_type_MAX;
>>> -    if (pC->eCurType == CURTYPE_TARANTOOL) {
>>> -        /*
>>> -         * Ephemeral spaces feature only one index
>>> -         * covering all fields, but ephemeral spaces
>>> -         * lack format. So, we can fetch type from
>>> -         * key parts.
>>> -         */
>>> -        if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
>>> -            field_type = pC->uc.pCursor->index->def->
>>> -                    key_def->parts[p2].type;
>>> -        } else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>>> -            field_type = pC->uc.pCursor->space->def->
>>> -                    fields[p2].type;
>>> -        }
>>> -    } else if (pC->eCurType == CURTYPE_SORTER) {
>>> +    if (pC->eCurType == CURTYPE_TARANTOOL)
>>> +        field_type = pC->uc.pCursor->space->def->fields[p2].type;
>>> +    else if (pC->eCurType == CURTYPE_SORTER)
>>>           field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
>>> -    }
>>>       struct Mem *default_val_mem =
>>>           pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
>>>       if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)
>> Shouldn't this be a part of the previous commit, which
>> introduces the ephemeral space format?
> They should, along with 3962. But, they have different milistones,
> I wasn't sure that it is right to put all three tickets in one
> patch-set.

But they already depend on each other. So what is a problem?

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Imeev Mergen]

On 4/12/20 5:02 PM, Vladislav Shpilevoy wrote:
>>>> --- a/src/box/sql/vdbe.c
>>>> +++ b/src/box/sql/vdbe.c
>>>> @@ -2694,23 +2694,10 @@ case OP_Column: {
>>>>            pC->cacheStatus = p->cacheCtr;
>>>>        }
>>>>        enum field_type field_type = field_type_MAX;
>>>> -    if (pC->eCurType == CURTYPE_TARANTOOL) {
>>>> -        /*
>>>> -         * Ephemeral spaces feature only one index
>>>> -         * covering all fields, but ephemeral spaces
>>>> -         * lack format. So, we can fetch type from
>>>> -         * key parts.
>>>> -         */
>>>> -        if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
>>>> -            field_type = pC->uc.pCursor->index->def->
>>>> -                    key_def->parts[p2].type;
>>>> -        } else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>>>> -            field_type = pC->uc.pCursor->space->def->
>>>> -                    fields[p2].type;
>>>> -        }
>>>> -    } else if (pC->eCurType == CURTYPE_SORTER) {
>>>> +    if (pC->eCurType == CURTYPE_TARANTOOL)
>>>> +        field_type = pC->uc.pCursor->space->def->fields[p2].type;
>>>> +    else if (pC->eCurType == CURTYPE_SORTER)
>>>>            field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
>>>> -    }
>>>>        struct Mem *default_val_mem =
>>>>            pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
>>>>        if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)
>>> Shouldn't this be a part of the previous commit, which
>>> introduces the ephemeral space format?
>> They should, along with 3962. But, they have different milistones,
>> I wasn't sure that it is right to put all three tickets in one
>> patch-set.
> But they already depend on each other. So what is a problem?
True. Should I resend the patch-set?

Re: [Tarantool-patches] [PATCH v2 1/1] sql: do not change order of inserted values

[Vladislav Shpilevoy]

On 12/04/2020 17:21, Imeev Mergen wrote:
> 
> On 4/12/20 5:02 PM, Vladislav Shpilevoy wrote:
>>>>> --- a/src/box/sql/vdbe.c
>>>>> +++ b/src/box/sql/vdbe.c
>>>>> @@ -2694,23 +2694,10 @@ case OP_Column: {
>>>>>            pC->cacheStatus = p->cacheCtr;
>>>>>        }
>>>>>        enum field_type field_type = field_type_MAX;
>>>>> -    if (pC->eCurType == CURTYPE_TARANTOOL) {
>>>>> -        /*
>>>>> -         * Ephemeral spaces feature only one index
>>>>> -         * covering all fields, but ephemeral spaces
>>>>> -         * lack format. So, we can fetch type from
>>>>> -         * key parts.
>>>>> -         */
>>>>> -        if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor) {
>>>>> -            field_type = pC->uc.pCursor->index->def->
>>>>> -                    key_def->parts[p2].type;
>>>>> -        } else if (pC->uc.pCursor->curFlags & BTCF_TaCursor) {
>>>>> -            field_type = pC->uc.pCursor->space->def->
>>>>> -                    fields[p2].type;
>>>>> -        }
>>>>> -    } else if (pC->eCurType == CURTYPE_SORTER) {
>>>>> +    if (pC->eCurType == CURTYPE_TARANTOOL)
>>>>> +        field_type = pC->uc.pCursor->space->def->fields[p2].type;
>>>>> +    else if (pC->eCurType == CURTYPE_SORTER)
>>>>>            field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
>>>>> -    }
>>>>>        struct Mem *default_val_mem =
>>>>>            pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
>>>>>        if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0)
>>>> Shouldn't this be a part of the previous commit, which
>>>> introduces the ephemeral space format?
>>> They should, along with 3962. But, they have different milistones,
>>> I wasn't sure that it is right to put all three tickets in one
>>> patch-set.
>> But they already depend on each other. So what is a problem?
> True. Should I resend the patch-set?

I guess Nikita anyway should take a look. So it would be good.
Better send them all as one patchset, I think. Since they can't
be applied separately.