Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: vdavydov@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v2 2/2] sql: define ephemeral space fields
Date: Tue, 17 Aug 2021 13:43:14 +0300	[thread overview]
Message-ID: <d9360d4e17f417b2286431159a4cd16714178fb5.1629196232.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1629196232.git.imeevma@gmail.com>

Hi! Thank you for the review! My answers, diffs and new patch below. I included
two diffs, one for renaming and one for removing key_info from ephemeral spaces.

On 16.08.2021 11:57, Vladimir Davydov wrote:
> On Sun, Aug 15, 2021 at 04:26:03PM +0300, Mergen Imeev wrote:
>> On Fri, Aug 13, 2021 at 06:12:29PM +0300, Vladimir Davydov wrote:
>>> On Fri, Aug 13, 2021 at 06:09:19AM +0300, imeevma@tarantool.org wrote:
>>>> diff --git a/src/box/sql.c b/src/box/sql.c
>>>> index c805a1e5c..2b95aed8d 100644
>>>> --- a/src/box/sql.c
>>>> +++ b/src/box/sql.c
>>>> @@ -243,114 +243,163 @@ tarantoolsqlCount(struct BtCursor *pCur)
>>>>  	return index_count(pCur->index, pCur->iter_type, NULL, 0);
>>>>  }
>>>>  
>>>> -struct space *
>>>> -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>>>> +struct space_info *
>>>> +sql_space_info_new(uint32_t field_count, uint32_t part_count)
>>>>  {
>>>> -	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;
>>>> +	assert(field_count > 0);
>>>> +	uint32_t info_size = sizeof(struct space_info);
>>>> +	uint32_t field_size = field_count * sizeof(enum field_type);
>>>> +	uint32_t colls_size = field_count * sizeof(uint32_t);
>>>> +	uint32_t parts_size = part_count * sizeof(uint32_t);
>>>> +	uint32_t size = info_size + field_size + colls_size + parts_size;
>>>> +
>>>> +	struct space_info *info = sqlDbMallocRawNN(sql_get(), size);
>>>> +	if (info == NULL) {
>>>> +		diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
>>>> +		return NULL;
>>>>  	}
>>>> +	info->types = (enum field_type *)((char *)info + info_size);
>>>> +	info->coll_ids = (uint32_t *)((char *)info->types + field_size);
>>>> +	info->parts = part_count == 0 ? NULL :
>>>> +		      (uint32_t *)((char *)info->coll_ids + colls_size);
>>>> +	info->field_count = field_count;
>>>> +	info->part_count = part_count;
>>>> +	info->sort_order = SORT_ORDER_ASC;
>>>>  
>>>> -	struct region *region = &fiber()->gc;
>>>> +	for (uint32_t i = 0; i < field_count; ++i) {
>>>> +		info->types[i] = FIELD_TYPE_SCALAR;
>>>> +		info->coll_ids[i] = COLL_NONE;
>>>> +	}
>>>> +	for (uint32_t i = 0; i < part_count; ++i)
>>>> +		info->parts[i] = i;
>>>> +	return info;
>>>> +}
>>>> +
>>>> +void
>>>> +sql_space_info_from_space_def(struct space_info *info,
>>>> +			      const struct space_def *def)
>>>
>>> Separating allocation from initialization only complicates the protocol:
>>>  - The caller must ensure that the arguments passed to alloc and init
>>>    are compatible.
>>>  - There's a window between alloc and init when the object is unusable.
>>>
>>> Please don't:
>>>
>>> 	struct sql_space_info *
>>> 	sql_space_info_new_from_space_def(...);
>>>
>>> 	struct sql_space_info *
>>> 	sql_space_info_new_from_index_def(...);
>>>
>> Fixed. There is still some places that cannot be fixed for now, since
>> sql_space_info may be modified during execution of some code.
>
> Please also rename sql_space_info_from_id_list to
> sql_space_info_new_from_id_list.
>

Fixed.

>>> struct space *
>>> sql_ephemeral_space_new(const struct sql_space_info *info);
>>>
>> I would like to do this, but we already have a function with such name. More
>> than that, this functions actually have no connection to ephemeral spaces,
>> the "spaces" it creates are only needed for creation of normal spaces, i.e.
>> it actually creates analogues of space_def. Not sure why space_def cannot be
>> used there.
>
> Then you should give those old functions adequate names.
> Please rename them in a separate patch and rebase.
>
Fixed, sent additional patch.

>>>> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
>>>> index b9107fccc..a3e551017 100644
>>>> --- a/src/box/sql/select.c
>>>> +++ b/src/box/sql/select.c
>>>> @@ -5879,6 +6047,23 @@ sqlSelect(Parse * pParse,		/* The parser context */
>>>>  				      sSort.reg_eph,
>>>>  				      nCols,
>>>>  				      0, (char *)key_info, P4_KEYINFO);
>>>> +		/*
>>>> +		 * We also should define space_info in case this opcode will not
>>>> +		 * be converted to SorterOpen.
>>>> +		 */
>>>> +		uint32_t part_count = sSort.pOrderBy->nExpr + 1;
>>>> +		struct space_info *info = sql_space_info_new(nCols, part_count);
>>>> +		if (info == NULL) {
>>>> +			pParse->is_aborted = true;
>>>> +			goto select_end;
>>>> +		}
>>>> +		key_info->info = info;
>>>> +		if (space_info_for_sorting(info, pParse, sSort.pOrderBy,
>>>> +					   pEList) != 0) {
>>>> +			pParse->is_aborted = true;
>>>> +			goto select_end;
>>>> +		}
>>>> +
>>>
>>> I don't understand why you can't pass sql_space_info instead of
>>> sql_key_info. AFAIU, you don't need key_info here at all anymore.
>>> Anyway, keeping a pointer to space_info in key_info looks bad.
>>> Can you avoid that?
>>>
>> The problem here is that OpenTEphemeral opcode can be replaced by SorterOpen
>> opcode. Also, its key_info could be moved to OP_Compare opcode. Actually, I
>> made a patch which also replaces sql_key_info by space_info for these two
>> opcodes, but space_info become too complicated. I decided to leave it for now.
>> I actually think that we should drop sorter and OP_Compare, but not sure that
>> we should do it now, before release. I will think of it a bit later, if I will
>> be given some time to rework this.
>
> I think you should convert space_info to key_info when changing
> the opcode then.

Done.


Diff 1:

diff --git a/src/box/sql.c b/src/box/sql.c
index 4340db622..e43a97c84 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -310,7 +310,7 @@ sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid)
 }
 
 struct space *
-sql_ephemeral_space_new_from_info(const struct sql_space_info *info)
+sql_ephemeral_space_new(const struct sql_space_info *info)
 {
 	uint32_t field_count = info->field_count;
 	uint32_t part_count = info->parts == NULL ? field_count :
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index cc7b6b0e3..341c202a2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -61,8 +61,8 @@ sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg)
 }
 
 static struct sql_space_info *
-sql_space_info_from_id_list(const struct space_def *def, int len,
-			    const struct IdList *list)
+sql_space_info_new_from_id_list(const struct space_def *def, int len,
+				const struct IdList *list)
 {
 	struct sql_space_info *info = sql_space_info_new(len + 1, 1);
 	if (info == NULL)
@@ -471,8 +471,9 @@ sqlInsert(Parse * pParse,	/* Parser context */
 			 * other parts.
 			 */
 			struct sql_space_info *info =
-				sql_space_info_from_id_list(space_def, nColumn,
-							    pColumn);
+				sql_space_info_new_from_id_list(space_def,
+								nColumn,
+								pColumn);
 			if (info == NULL) {
 				pParse->is_aborted = true;
 				goto insert_cleanup;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2564eda8a..4c013478b 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4113,7 +4113,7 @@ sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid);
  * key parts is taken from the space_info structure.
  */
 struct space *
-sql_ephemeral_space_new_from_info(const struct sql_space_info *info);
+sql_ephemeral_space_new(const struct sql_space_info *info);
 
 /**
  * Check if the function implements LIKE-style comparison & if it
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 0facd75c7..7cef1ca77 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2393,7 +2393,7 @@ case OP_OpenTEphemeral: {
 				      pOp->p4.key_info->info :
 				      pOp->p4.space_info;
 	assert(info != NULL);
-	struct space *space = sql_ephemeral_space_new_from_info(info);
+	struct space *space = sql_ephemeral_space_new(info);
 
 	if (space == NULL)
 		goto abort_due_to_error;




Diff 2:


diff --git a/src/box/sql.c b/src/box/sql.c
index e43a97c84..d15159d6e 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -251,7 +251,9 @@ sql_space_info_new(uint32_t field_count, uint32_t part_count)
 	uint32_t field_size = field_count * sizeof(enum field_type);
 	uint32_t colls_size = field_count * sizeof(uint32_t);
 	uint32_t parts_size = part_count * sizeof(uint32_t);
-	uint32_t size = info_size + field_size + colls_size + parts_size;
+	uint32_t sort_orders_size = part_count * sizeof(enum sort_order);
+	uint32_t size = info_size + field_size + colls_size + parts_size +
+			sort_orders_size;
 
 	struct sql_space_info *info = sqlDbMallocRawNN(sql_get(), size);
 	if (info == NULL) {
@@ -262,16 +264,20 @@ sql_space_info_new(uint32_t field_count, uint32_t part_count)
 	info->coll_ids = (uint32_t *)((char *)info->types + field_size);
 	info->parts = part_count == 0 ? NULL :
 		      (uint32_t *)((char *)info->coll_ids + colls_size);
+	info->sort_orders = part_count == 0 ? NULL :
+			    (uint32_t *)((char *)info->parts + parts_size);
 	info->field_count = field_count;
 	info->part_count = part_count;
-	info->sort_order = SORT_ORDER_ASC;
 
 	for (uint32_t i = 0; i < field_count; ++i) {
 		info->types[i] = FIELD_TYPE_SCALAR;
 		info->coll_ids[i] = COLL_NONE;
 	}
-	for (uint32_t i = 0; i < part_count; ++i)
+	for (uint32_t i = 0; i < part_count; ++i) {
 		info->parts[i] = i;
+		info->sort_orders[i] = SORT_ORDER_ASC;
+
+	}
 	return info;
 }
 
@@ -357,7 +363,7 @@ sql_ephemeral_space_new(const struct sql_space_info *info)
 		parts[i].nullable_action = ON_CONFLICT_ACTION_NONE;
 		parts[i].is_nullable = true;
 		parts[i].exclude_null = false;
-		parts[i].sort_order = info->sort_order;
+		parts[i].sort_order = SORT_ORDER_ASC;
 		parts[i].path = NULL;
 		parts[i].type = info->types[j];
 		parts[i].coll_id = info->coll_ids[j];
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 97962c56f..021e0ebd5 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -130,7 +130,6 @@ sql_space_info_new_from_expr_list(struct Parse *parser, struct ExprList *list,
 		info->types[i] = type;
 		info->coll_ids[i] = coll_id;
 	}
-	info->sort_order = list->a[0].sort_order;
 	if (has_rowid)
 		info->types[list->nExpr] = FIELD_TYPE_INTEGER;
 	return info;
@@ -172,7 +171,6 @@ sql_space_info_new_from_order_by(struct Parse *parser, struct Select *select,
 				sqlExprAddCollateString(parser, expr, name);
 		}
 	}
-	info->sort_order = order_by->a[0].sort_order;
 	info->types[order_by->nExpr] = FIELD_TYPE_INTEGER;
 	info->types[order_by->nExpr + 1] = FIELD_TYPE_VARBINARY;
 	return info;
@@ -220,6 +218,7 @@ sql_space_info_new_for_sorting(struct Parse *parser, struct ExprList *order_by,
 			return NULL;
 		info->types[k] = type;
 		info->coll_ids[k] = coll_id;
+		info->sort_orders[k] = order_by->a[k + start].sort_order;
 	}
 	if (has_rowid)
 		info->types[k++] = FIELD_TYPE_INTEGER;
@@ -241,10 +240,36 @@ sql_space_info_new_for_sorting(struct Parse *parser, struct ExprList *order_by,
 		++k;
 	}
 	assert(k == (int)field_count);
-	info->sort_order = order_by->a[0].sort_order;
 	return info;
 }
 
+static struct sql_key_info *
+sql_key_info_new_from_space_info(const struct sql_space_info *info)
+{
+	uint32_t part_count = info->part_count - 1;
+	assert(part_count > 0);
+	uint32_t size = sizeof(struct sql_key_info) +
+			part_count * sizeof(struct key_part_def);
+	struct sql_key_info *key_info = sqlDbMallocRawNN(sql_get(), size);
+	if (key_info == NULL)
+		return NULL;
+	key_info->db = sql_get();
+	key_info->key_def = NULL;
+	key_info->refs = 1;
+	key_info->part_count = part_count;
+	for (uint32_t i = 0; i < part_count; ++i) {
+		key_info->parts[i].fieldno = i;
+		key_info->parts[i].nullable_action = ON_CONFLICT_ACTION_ABORT;
+		key_info->parts[i].is_nullable = false;
+		key_info->parts[i].exclude_null = false;
+		key_info->parts[i].sort_order = info->sort_orders[i];
+		key_info->parts[i].path = NULL;
+		key_info->parts[i].type = info->types[i];
+		key_info->parts[i].coll_id = info->coll_ids[i];
+	}
+	return key_info;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -965,12 +990,14 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 		if (pParse->db->mallocFailed)
 			return;
 		pOp->p2 = nKey + nData;
-		struct sql_key_info *key_info = pOp->p4.key_info;
+		assert(pOp->opcode == OP_OpenTEphemeral ||
+		       pOp->opcode == OP_SorterOpen);
+		struct sql_key_info *key_info =
+			pOp->opcode == OP_SorterOpen ? pOp->p4.key_info :
+			sql_key_info_new_from_space_info(pOp->p4.space_info);
 		for (uint32_t i = 0; i < key_info->part_count; i++)
 			key_info->parts[i].sort_order = SORT_ORDER_ASC;
 		sqlVdbeChangeP4(v, -1, (char *)key_info, P4_KEYINFO);
-		assert(pOp->opcode == OP_OpenTEphemeral ||
-		       pOp->opcode == OP_SorterOpen);
 		if (pOp->opcode == OP_SorterOpen) {
 			pOp->p4.key_info =
 				sql_expr_list_to_key_info(pParse,
@@ -986,8 +1013,8 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 				pParse->is_aborted = true;
 				return;
 			}
-			pOp->p4.space_info = info;
-			pOp->p4type = P4_DYNAMIC;
+			sqlVdbeChangeP4(v, pSort->addrSortIndex, (char *)info,
+					P4_DYNAMIC);
 		}
 		addrJmp = sqlVdbeCurrentAddr(v);
 		sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
@@ -1209,11 +1236,6 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 			uint32_t excess_field_count = 0;
 			struct VdbeOp *op = sqlVdbeGetOp(v,
 							 pSort->addrSortIndex);
-			struct sql_space_info *info;
-			if (op->p4type == P4_KEYINFO)
-				info = op->p4.key_info->info;
-			else
-				info = op->p4.space_info;
 			for (i = pSort->nOBSat; i < pSort->pOrderBy->nExpr;
 			     i++) {
 				int j = pSort->pOrderBy->a[i].u.x.iOrderByCol;
@@ -1223,6 +1245,9 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 				excess_field_count++;
 				pEList->a[j - 1].u.x.iOrderByCol =
 					(uint16_t)(i + 1 - pSort->nOBSat);
+				if (op->opcode != OP_OpenTEphemeral)
+					continue;
+				struct sql_space_info *info = op->p4.space_info;
 				--info->field_count;
 				for (int k = j; k < pEList->nExpr; ++k) {
 					int n = k + pSort->pOrderBy->nExpr + 1;
@@ -1231,12 +1256,11 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 						info->coll_ids[n];
 				}
 			}
-			struct VdbeOp *open_eph_op =
-				sqlVdbeGetOp(v, pSort->addrSortIndex);
-			assert(open_eph_op->p2 - excess_field_count > 0);
-			sqlVdbeChangeP2(v, pSort->addrSortIndex,
-					open_eph_op->p2 -
-					excess_field_count);
+			if (op->opcode != OP_OpenTEphemeral) {
+				assert(op->p2 - excess_field_count > 0);
+				sqlVdbeChangeP2(v, pSort->addrSortIndex,
+						op->p2 - excess_field_count);
+			}
 			regOrig = 0;
 			assert(eDest == SRT_Set || eDest == SRT_Mem
 			       || eDest == SRT_Coroutine
@@ -1594,11 +1618,9 @@ sql_key_info_new(sql *db, uint32_t part_count)
 		return NULL;
 	}
 	key_info->db = db;
-	key_info->info = NULL;
 	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;
@@ -1613,25 +1635,6 @@ sql_key_info_new(sql *db, uint32_t part_count)
 	return key_info;
 }
 
-struct sql_key_info *
-sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def)
-{
-	struct sql_key_info *key_info = sqlDbMallocRawNN(db,
-				sql_key_info_sizeof(key_def->part_count));
-	if (key_info == NULL) {
-		sqlOomFault(db);
-		return NULL;
-	}
-	key_info->db = db;
-	key_info->info = NULL;
-	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;
-}
-
 struct sql_key_info *
 sql_key_info_ref(struct sql_key_info *key_info)
 {
@@ -1649,7 +1652,6 @@ sql_key_info_unref(struct sql_key_info *key_info)
 	if (--key_info->refs == 0) {
 		if (key_info->key_def != NULL)
 			key_def_delete(key_info->key_def);
-		sqlDbFree(key_info->db, key_info->info);
 		sqlDbFree(key_info->db, key_info);
 	}
 }
@@ -6059,29 +6061,8 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	 * that change.
 	 */
 	if (sSort.pOrderBy) {
-		struct sql_key_info *key_info =
-			sql_expr_list_to_key_info(pParse, sSort.pOrderBy, 0);
 		sSort.reg_eph = ++pParse->nMem;
 		sSort.iECursor = pParse->nTab++;
-		/* Number of columns in transient table equals to number of columns in
-		 * SELECT statement plus number of columns in ORDER BY statement
-		 * and plus one column for ID.
-		 */
-		int nCols = pEList->nExpr + sSort.pOrderBy->nExpr + 1;
-		if (key_info->parts[0].sort_order == SORT_ORDER_DESC) {
-			sSort.sortFlags |= SORTFLAG_DESC;
-		}
-		/*
-		 * We need to defined both sql_key_info and sql_space_info since
-		 * this opcode may change to OP_SorterOpen, which uses
-		 * sql_key_info. This is also the reason why we should give p2
-		 * for this opcode.
-		 */
-		sSort.addrSortIndex =
-		    sqlVdbeAddOp4(v, OP_OpenTEphemeral,
-				      sSort.reg_eph,
-				      nCols,
-				      0, (char *)key_info, P4_KEYINFO);
 		struct sql_space_info *info =
 			sql_space_info_new_for_sorting(pParse, sSort.pOrderBy,
 						       pEList, 0, true);
@@ -6089,7 +6070,11 @@ sqlSelect(Parse * pParse,		/* The parser context */
 			pParse->is_aborted = true;
 			goto select_end;
 		}
-		key_info->info = info;
+		if (info->sort_orders[0] == SORT_ORDER_DESC)
+			sSort.sortFlags |= SORTFLAG_DESC;
+		sSort.addrSortIndex =
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, sSort.reg_eph, 0, 0,
+				      (char *)info, P4_DYNAMIC);
 
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
@@ -6122,8 +6107,15 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	}
 	computeLimitRegisters(pParse, p, iEnd);
 	if (p->iLimit == 0 && sSort.addrSortIndex >= 0) {
+		struct VdbeOp *op = sqlVdbeGetOp(v, sSort.addrSortIndex);
+		struct sql_key_info *key_info =
+			sql_key_info_new_from_space_info(op->p4.space_info);
 		sqlVdbeChangeOpcode(v, sSort.addrSortIndex, OP_SorterOpen);
 		sqlVdbeChangeP1(v, sSort.addrSortIndex, sSort.iECursor);
+		sqlVdbeChangeP2(v, sSort.addrSortIndex,
+				op->p4.space_info->field_count);
+		sqlVdbeChangeP4(v, sSort.addrSortIndex, (char *)key_info,
+				P4_KEYINFO);
 		sqlVdbeChangeToNoop(v, sSort.addrSortIndex + 1);
 		sSort.sortFlags |= SORTFLAG_UseSorter;
 	}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 4c013478b..540c3a2ff 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4006,8 +4006,6 @@ sql_analysis_load(struct sql *db);
  */
 struct sql_key_info {
 	sql *db;
-	/** Informations about all field types and key parts. */
-	struct sql_space_info *info;
 	/**
 	 * Key definition created from this object,
 	 * see sql_key_info_to_key_def().
@@ -4015,8 +4013,6 @@ 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. */
@@ -4030,12 +4026,6 @@ struct sql_key_info {
 struct sql_key_info *
 sql_key_info_new(sql *db, uint32_t part_count);
 
-/**
- * Allocate a key_info object from the given key definition.
- */
-struct sql_key_info *
-sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def);
-
 /**
  * Increment the reference counter of a key_info object.
  */
@@ -4071,6 +4061,8 @@ struct sql_space_info {
 	 * consists of all fields in sequential order.
 	 */
 	uint32_t *parts;
+	/** Sort order of index. */
+	enum sort_order *sort_orders;
 	/** Number of fields of ephemetal space. */
 	uint32_t field_count;
 	/**
@@ -4078,8 +4070,6 @@ struct sql_space_info {
 	 * is also NULL.
 	 */
 	uint32_t part_count;
-	/** Sort order of index. */
-	enum sort_order sort_order;
 };
 
 /**
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 7cef1ca77..ce30afc5a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2389,9 +2389,8 @@ open_cursor_set_hints:
 case OP_OpenTEphemeral: {
 	assert(pOp->p1 >= 0);
 
-	struct sql_space_info *info = pOp->p4type == P4_KEYINFO ?
-				      pOp->p4.key_info->info :
-				      pOp->p4.space_info;
+	assert(pOp->p4type == P4_DYNAMIC || pOp->p4type == P4_STATIC);
+	struct sql_space_info *info = pOp->p4.space_info;
 	assert(info != NULL);
 	struct space *space = sql_ephemeral_space_new(info);
 


New patch:


commit d9360d4e17f417b2286431159a4cd16714178fb5
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Aug 10 08:27:14 2021 +0300

    sql: define ephemeral space fields
    
    Prior to this patch, most ephemeral space fields were defined using the
    SCALAR type. After this patch, almost all fields will be properly
    defined. However, there are still cases where SCALAR will be set by
    default. For example, in IN, where rules is still not defined (#4692).
    Another example is when a field is not resolved because of a too complex
    query.
    
    Part of #6213

diff --git a/src/box/sql.c b/src/box/sql.c
index 0e93aa7bb..d15159d6e 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -243,114 +243,161 @@ tarantoolsqlCount(struct BtCursor *pCur)
 	return index_count(pCur->index, pCur->iter_type, NULL, 0);
 }
 
-struct space *
-sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
+struct sql_space_info *
+sql_space_info_new(uint32_t field_count, uint32_t part_count)
 {
-	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;
+	assert(field_count > 0);
+	uint32_t info_size = sizeof(struct sql_space_info);
+	uint32_t field_size = field_count * sizeof(enum field_type);
+	uint32_t colls_size = field_count * sizeof(uint32_t);
+	uint32_t parts_size = part_count * sizeof(uint32_t);
+	uint32_t sort_orders_size = part_count * sizeof(enum sort_order);
+	uint32_t size = info_size + field_size + colls_size + parts_size +
+			sort_orders_size;
+
+	struct sql_space_info *info = sqlDbMallocRawNN(sql_get(), size);
+	if (info == NULL) {
+		diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
+		return NULL;
 	}
+	info->types = (enum field_type *)((char *)info + info_size);
+	info->coll_ids = (uint32_t *)((char *)info->types + field_size);
+	info->parts = part_count == 0 ? NULL :
+		      (uint32_t *)((char *)info->coll_ids + colls_size);
+	info->sort_orders = part_count == 0 ? NULL :
+			    (uint32_t *)((char *)info->parts + parts_size);
+	info->field_count = field_count;
+	info->part_count = part_count;
 
-	struct region *region = &fiber()->gc;
+	for (uint32_t i = 0; i < field_count; ++i) {
+		info->types[i] = FIELD_TYPE_SCALAR;
+		info->coll_ids[i] = COLL_NONE;
+	}
+	for (uint32_t i = 0; i < part_count; ++i) {
+		info->parts[i] = i;
+		info->sort_orders[i] = SORT_ORDER_ASC;
+
+	}
+	return info;
+}
+
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def)
+{
+	uint32_t field_count = def->field_count + 1;
+	struct sql_space_info *info = sql_space_info_new(field_count, 0);
+	if (info == NULL)
+		return NULL;
+	for (uint32_t i = 0; i < def->field_count; ++i) {
+		info->types[i] = def->fields[i].type;
+		info->coll_ids[i] = def->fields[i].coll_id;
+	}
+	/* Add one more field for rowid. */
+	info->types[def->field_count] = FIELD_TYPE_INTEGER;
+	return info;
+}
+
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid)
+{
+	uint32_t field_count = def->key_def->part_count;
+	if (has_rowid)
+		++field_count;
+	struct sql_space_info *info = sql_space_info_new(field_count, 0);
+	if (info == NULL)
+		return NULL;
+	for (uint32_t i = 0; i < def->key_def->part_count; ++i) {
+		info->types[i] = def->key_def->parts[i].type;
+		info->coll_ids[i] = def->key_def->parts[i].coll_id;
+	}
+	if (has_rowid)
+		info->types[def->key_def->part_count] = FIELD_TYPE_INTEGER;
+	return info;
+}
+
+struct space *
+sql_ephemeral_space_new(const struct sql_space_info *info)
+{
+	uint32_t field_count = info->field_count;
+	uint32_t part_count = info->parts == NULL ? field_count :
+			      info->part_count;
+	uint32_t parts_indent = field_count * sizeof(struct field_def);
+	uint32_t names_indent = part_count * sizeof(struct key_part_def) +
+				parts_indent;
 	/*
-	 * Name of the fields will be "_COLUMN_1", "_COLUMN_2"
-	 * and so on. Due to this, length of each name is no more
-	 * than strlen("_COLUMN_") plus length of UINT32_MAX
-	 * turned to string, which is 10 and plus 1 for \0.
+	 * Name of the fields will be "_COLUMN_1", "_COLUMN_2" and so on. Due to
+	 * this, length of each name is no more than 19 == strlen("_COLUMN_")
+	 * plus length of UINT32_MAX turned to string, which is 10 and plus 1
+	 * for '\0'.
 	 */
-	uint32_t name_len = strlen("_COLUMN_") + 11;
-	uint32_t size = field_count * (sizeof(struct field_def) + name_len) +
-			part_count * sizeof(struct key_part_def);
+	uint32_t size = names_indent + field_count * 19;
+
+	struct region *region = &fiber()->gc;
+	size_t svp = region_used(region);
 	struct field_def *fields = region_aligned_alloc(region, size,
 							alignof(fields[0]));
 	if (fields == NULL) {
 		diag_set(OutOfMemory, size, "region_aligned_alloc", "fields");
 		return NULL;
 	}
-	struct key_part_def *ephemer_key_parts =
-		(void *)fields + field_count * sizeof(struct field_def);
-	static_assert(alignof(*fields) == alignof(*ephemer_key_parts),
-		      "allocated in one block, and should have the same "
-		      "alignment");
-	char *names = (char *)ephemer_key_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;
-		names += name_len;
-		sprintf(field->name, "_COLUMN_%d", i);
-		field->is_nullable = true;
-		field->nullable_action = ON_CONFLICT_ACTION_NONE;
-		field->default_value = NULL;
-		field->default_value_expr = NULL;
-		if (def != NULL && i < def->part_count) {
-			assert(def->parts[i].type < field_type_MAX);
-			field->type = def->parts[i].type;
-			field->coll_id = def->parts[i].coll_id;
-		} else {
-			field->coll_id = COLL_NONE;
-			field->type = FIELD_TYPE_SCALAR;
-		}
+	struct key_part_def *parts = (struct key_part_def *)((char *)fields +
+							     parts_indent);
+	static_assert(alignof(*fields) == alignof(*parts), "allocated in one "
+		      "block, and should have the same alignment");
+	char *names = (char *)fields + names_indent;
+
+	for (uint32_t i = 0; i < info->field_count; ++i) {
+		fields[i].name = names;
+		sprintf(names, "_COLUMN_%d", i);
+		names += strlen(fields[i].name) + 1;
+		fields[i].is_nullable = true;
+		fields[i].nullable_action = ON_CONFLICT_ACTION_NONE;
+		fields[i].default_value = NULL;
+		fields[i].default_value_expr = NULL;
+		fields[i].type = info->types[i];
+		fields[i].coll_id = info->coll_ids[i];
 	}
-
 	for (uint32_t i = 0; i < part_count; ++i) {
-		struct key_part_def *part = &ephemer_key_parts[i];
-		/*
-		 * In case we need to save initial order of
-		 * inserted into ephemeral space rows we use rowid
-		 * as the only part of PK. If ephemeral space has
-		 * a rowid, it is always the last column.
-		 */
-		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->exclude_null = false;
-		part->sort_order = SORT_ORDER_ASC;
-		part->path = NULL;
-		part->type = fields[j].type;
-		part->coll_id = fields[j].coll_id;
+		uint32_t j = info->parts == NULL ? i : info->parts[i];
+		parts[i].fieldno = j;
+		parts[i].nullable_action = ON_CONFLICT_ACTION_NONE;
+		parts[i].is_nullable = true;
+		parts[i].exclude_null = false;
+		parts[i].sort_order = SORT_ORDER_ASC;
+		parts[i].path = NULL;
+		parts[i].type = info->types[j];
+		parts[i].coll_id = info->coll_ids[j];
 	}
-	struct key_def *ephemer_key_def = key_def_new(ephemer_key_parts,
-						      part_count, false);
-	if (ephemer_key_def == NULL)
+
+	struct key_def *key_def = key_def_new(parts, part_count, false);
+	if (key_def == NULL)
 		return NULL;
 
-	struct index_def *ephemer_index_def =
-		index_def_new(0, 0, "ephemer_idx", strlen("ephemer_idx"), TREE,
-			      &index_opts_default, ephemer_key_def, NULL);
-	key_def_delete(ephemer_key_def);
-	if (ephemer_index_def == NULL)
+	const char *name = "ephemer_idx";
+	struct index_def *index_def = index_def_new(0, 0, name, strlen(name),
+						    TREE, &index_opts_default,
+						    key_def, NULL);
+	key_def_delete(key_def);
+	if (index_def == NULL)
 		return NULL;
 
 	struct rlist key_list;
 	rlist_create(&key_list);
-	rlist_add_entry(&key_list, ephemer_index_def, link);
+	rlist_add_entry(&key_list, index_def, link);
 
-	struct space_def *ephemer_space_def =
-		space_def_new_ephemeral(field_count, fields);
-	if (ephemer_space_def == NULL) {
-		index_def_delete(ephemer_index_def);
+	struct space_def *space_def = space_def_new_ephemeral(field_count,
+							      fields);
+	if (space_def == NULL) {
+		index_def_delete(index_def);
 		return NULL;
 	}
 
-	struct space *ephemer_new_space = space_new_ephemeral(ephemer_space_def,
-							      &key_list);
-	index_def_delete(ephemer_index_def);
-	space_def_delete(ephemer_space_def);
+	struct space *space = space_new_ephemeral(space_def, &key_list);
+	index_def_delete(index_def);
+	space_def_delete(space_def);
+	region_truncate(region, svp);
 
-	return ephemer_new_space;
+	return space;
 }
 
 int tarantoolsqlEphemeralInsert(struct space *space, const char *tuple,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5226dd6ea..b82a6874e 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -224,12 +224,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		 * is held in ephemeral table, there is no PK for
 		 * it, so columns should be loaded manually.
 		 */
-		struct sql_key_info *pk_info = NULL;
 		int reg_eph = ++parse->nMem;
 		int reg_pk = parse->nMem + 1;
-		int pk_len;
+		int pk_len = is_view ? space->def->field_count + 1 :
+			     space->index[0]->def->key_def->part_count;
 		int eph_cursor = parse->nTab++;
 		int addr_eph_open = sqlVdbeCurrentAddr(v);
+		struct sql_space_info *info;
 		if (is_view) {
 			/*
 			 * At this stage SELECT is already materialized
@@ -249,22 +250,20 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 			 * account that id field as well. That's why pk_len
 			 * has one field more than view format.
 			 */
-			pk_len = space->def->field_count + 1;
-			parse->nMem += pk_len;
-			sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
-					  pk_len);
+			info = sql_space_info_new_from_space_def(space->def);
 		} else {
                         assert(space->index_count > 0);
-                        pk_info = sql_key_info_new_from_key_def(db,
-					space->index[0]->def->key_def);
-                        if (pk_info == NULL)
-                                goto delete_from_cleanup;
-                        pk_len = pk_info->part_count;
-                        parse->nMem += pk_len;
-			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
-					  pk_len, 0,
-					  (char *)pk_info, P4_KEYINFO);
+			struct index_def *index_def = space->index[0]->def;
+			info = sql_space_info_new_from_index_def(index_def,
+								 false);
 		}
+		if (info == NULL) {
+			parse->is_aborted = true;
+			goto delete_from_cleanup;
+		}
+		parse->nMem += pk_len;
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+			      P4_DYNAMIC);
 
 		/* Construct a query to find the primary key for
 		 * every row to be deleted, based on the WHERE
@@ -295,8 +294,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 
 		/* Extract the primary key for the current row */
 		if (!is_view) {
-			struct key_part_def *part = pk_info->parts;
-			for (int i = 0; i < pk_len; i++, part++) {
+			struct key_def *def = space->index[0]->def->key_def;
+			for (int i = 0; i < pk_len; i++) {
+				struct key_part *part = &def->parts[i];
 				sqlVdbeAddOp3(v, OP_Column, tab_cursor,
 					      part->fieldno, reg_pk + i);
 			}
@@ -373,7 +373,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		if (one_pass != ONEPASS_OFF) {
 			/* OP_Found will use an unpacked key. */
 			assert(key_len == pk_len);
-			assert(pk_info != NULL || space->def->opts.is_view);
 			sqlVdbeAddOp4Int(v, OP_NotFound, tab_cursor,
 					     addr_bypass, reg_key, key_len);
 
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 6f40183ac..8902c648f 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2740,7 +2740,6 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 
 	switch (pExpr->op) {
 	case TK_IN:{
-			int addr;	/* Address of OP_OpenEphemeral instruction */
 			Expr *pLeft = pExpr->pLeft;	/* the LHS of the IN operator */
 			int nVal;	/* Size of vector pLeft */
 
@@ -2761,13 +2760,14 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 			 */
 			pExpr->iTable = pParse->nTab++;
 			int reg_eph = ++pParse->nMem;
-			addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral,
-						 reg_eph, nVal);
+			struct sql_space_info *info =
+				sql_space_info_new(nVal, 0);
+			if (info == NULL)
+				return 0;
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+				      (char *)info, P4_DYNAMIC);
 			sqlVdbeAddOp3(v, OP_IteratorOpen, pExpr->iTable, 0,
 					  reg_eph);
-			struct sql_key_info *key_info = sql_key_info_new(pParse->db, nVal);
-			if (key_info == NULL)
-				return 0;
 
 			if (ExprHasProperty(pExpr, EP_xIsSelect)) {
 				/* Case 1:     expr IN (SELECT ...)
@@ -2797,7 +2797,6 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 					    (pParse, pSelect, &dest)) {
 						sqlDbFree(pParse->db,
 							      dest.dest_type);
-						sql_key_info_unref(key_info);
 						return 0;
 					}
 					sqlDbFree(pParse->db,
@@ -2809,7 +2808,7 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 						    sqlVectorFieldSubexpr
 						    (pLeft, i);
 						if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr,
-										&key_info->parts[i].coll_id) != 0)
+										&info->coll_ids[i]) != 0)
 							return 0;
 					}
 				}
@@ -2829,7 +2828,7 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 				bool unused;
 				struct coll *unused_coll;
 				if (sql_expr_coll(pParse, pExpr->pLeft, &unused,
-						  &key_info->parts[0].coll_id,
+						  &info->coll_ids[0],
 						  &unused_coll) != 0)
 					return 0;
 
@@ -2861,8 +2860,6 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 				sqlReleaseTempReg(pParse, r1);
 				sqlReleaseTempReg(pParse, r2);
 			}
-			sqlVdbeChangeP4(v, addr, (void *)key_info,
-					    P4_KEYINFO);
 			break;
 		}
 
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 21b4f2407..341c202a2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -60,6 +60,27 @@ sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg)
 			  (char *)colls_type, P4_DYNAMIC);
 }
 
+static struct sql_space_info *
+sql_space_info_new_from_id_list(const struct space_def *def, int len,
+				const struct IdList *list)
+{
+	struct sql_space_info *info = sql_space_info_new(len + 1, 1);
+	if (info == NULL)
+		return NULL;
+	assert(len > 0 && len <= (int)def->field_count);
+	struct field_def *fields = def->fields;
+	for (int i = 0; i < len; ++i) {
+		int j = list == NULL ? i : list->a[i].idx;
+		info->types[i] =  fields[j].type;
+		info->coll_ids[i] =  fields[j].coll_id;
+	}
+	/* Add one more field for rowid. */
+	info->types[len] = FIELD_TYPE_INTEGER;
+	/* Set rowid as the only part of primary index. */
+	info->parts[0] = len;
+	return info;
+}
+
 /**
  * In SQL table can be created with AUTOINCREMENT.
  * In Tarantool it can be detected as primary key which consists
@@ -442,34 +463,30 @@ 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.
+			 * Order of inserted values is important since it is
+			 * possible, that NULL will be inserted in field with
+			 * AUTOINCREMENT. So, the first part of key should be
+			 * rowid. Since each rowid is unique, we do not need any
+			 * other parts.
 			 */
-			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;
-				sqlVdbeChangeP4(v, -1, (void *)key_info,
-					        P4_KEYINFO);
+			struct sql_space_info *info =
+				sql_space_info_new_from_id_list(space_def,
+								nColumn,
+								pColumn);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				goto insert_cleanup;
 			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+				      (char *)info, P4_DYNAMIC);
 			addrL = sqlVdbeAddOp1(v, OP_Yield, dest.iSDParm);
 			VdbeCoverage(v);
 			sqlVdbeAddOp2(v, OP_NextIdEphemeral, reg_eph,
 					  regCopy + nColumn);
 			sqlVdbeAddOp3(v, OP_Copy, regFromSelect, regCopy, nColumn-1);
+			sqlVdbeAddOp4(v, OP_ApplyType, regCopy, nColumn + 1, 0,
+				      (char *)info->types, P4_STATIC);
 			sqlVdbeAddOp3(v, OP_MakeRecord, regCopy,
 					  nColumn + 1, regRec);
 			/* Set flag to save memory allocating one by malloc. */
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 7cfe60db6..021e0ebd5 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -99,6 +99,177 @@ struct SortCtx {
 #define SORTFLAG_UseSorter  0x01	/* Use SorterOpen instead of OpenEphemeral */
 #define SORTFLAG_DESC 0xF0
 
+static inline uint32_t
+multi_select_coll_seq(struct Parse *parser, struct Select *p, int n);
+
+static struct sql_space_info *
+sql_space_info_new_from_expr_list(struct Parse *parser, struct ExprList *list,
+				  bool has_rowid)
+{
+	int n = list->nExpr;
+	if (has_rowid)
+		++n;
+	struct sql_space_info *info = sql_space_info_new(n, 0);
+	if (info == NULL)
+		return NULL;
+	for (int i = 0; i < list->nExpr; ++i) {
+		bool b;
+		struct coll *coll;
+		struct Expr *expr = list->a[i].pExpr;
+		enum field_type type = sql_expr_type(expr);
+		/*
+		 * Type ANY could mean that field was unresolved. We have no way
+		 * but to set it as SCALAR, however this could lead to
+		 * unexpected change of type.
+		 */
+		if (type == FIELD_TYPE_ANY)
+			type = FIELD_TYPE_SCALAR;
+		uint32_t coll_id;
+		if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
+			return NULL;
+		info->types[i] = type;
+		info->coll_ids[i] = coll_id;
+	}
+	if (has_rowid)
+		info->types[list->nExpr] = FIELD_TYPE_INTEGER;
+	return info;
+}
+
+static struct sql_space_info *
+sql_space_info_new_from_order_by(struct Parse *parser, struct Select *select,
+				 struct ExprList *order_by)
+{
+	int n = order_by->nExpr + 2;
+	struct sql_space_info *info = sql_space_info_new(n, 0);
+	if (info == NULL)
+		return NULL;
+	for (int i = 0; i < order_by->nExpr; ++i) {
+		struct Expr *expr = order_by->a[i].pExpr;
+		enum field_type type = sql_expr_type(expr);
+		/*
+		 * Type ANY could mean that field was unresolved. We have no way
+		 * but to set it as SCALAR, however this could lead to
+		 * unexpected change of type.
+		 */
+		if (type == FIELD_TYPE_ANY)
+			type = FIELD_TYPE_SCALAR;
+		info->types[i] = type;
+		uint32_t *id = &info->coll_ids[i];
+		if ((expr->flags & EP_Collate) != 0) {
+			bool b;
+			struct coll *coll;
+			if (sql_expr_coll(parser, expr, &b, id, &coll) != 0)
+				return NULL;
+			continue;
+		}
+		uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
+		info->coll_ids[i] = multi_select_coll_seq(parser, select,
+							  fieldno);
+		if (info->coll_ids[i] != COLL_NONE) {
+			const char *name = coll_by_id(info->coll_ids[i])->name;
+			order_by->a[i].pExpr =
+				sqlExprAddCollateString(parser, expr, name);
+		}
+	}
+	info->types[order_by->nExpr] = FIELD_TYPE_INTEGER;
+	info->types[order_by->nExpr + 1] = FIELD_TYPE_VARBINARY;
+	return info;
+}
+
+static struct sql_space_info *
+sql_space_info_new_for_sorting(struct Parse *parser, struct ExprList *order_by,
+			       struct ExprList *list, int start, bool has_rowid)
+{
+	/*
+	 * Index consist of fields that were not included into index plus rowid,
+	 * is has_rowid is TRUE.
+	 */
+	uint32_t part_count = order_by->nExpr - start;
+	if (has_rowid)
+		++part_count;
+	/*
+	 * Number of fields is number of parts in index plus number of fields
+	 * that is not appear in ORDER BY, but were in SELECT.
+	 */
+	uint32_t field_count = part_count;
+	/* If iOrderByCol != 0 than fields appear in ORDER BY. */
+	for (int i = 0; i < list->nExpr; ++i)
+		field_count += list->a[i].u.x.iOrderByCol == 0 ? 1 : 0;
+
+	struct sql_space_info *info =
+		sql_space_info_new(field_count, part_count);
+	if (info == NULL)
+		return NULL;
+	int k;
+	for (k = 0; k < order_by->nExpr - start; ++k) {
+		bool b;
+		struct coll *coll;
+		struct Expr *expr = order_by->a[k + start].pExpr;
+		enum field_type type = sql_expr_type(expr);
+		/*
+		 * Type ANY could mean that field was unresolved. We have no way
+		 * but to set it as SCALAR, however this could lead to
+		 * unexpected change of type.
+		 */
+		if (type == FIELD_TYPE_ANY)
+			type = FIELD_TYPE_SCALAR;
+		uint32_t coll_id;
+		if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
+			return NULL;
+		info->types[k] = type;
+		info->coll_ids[k] = coll_id;
+		info->sort_orders[k] = order_by->a[k + start].sort_order;
+	}
+	if (has_rowid)
+		info->types[k++] = FIELD_TYPE_INTEGER;
+	assert(k == (int)part_count);
+	for (int i = 0; i < list->nExpr; ++i) {
+		if (list->a[i].u.x.iOrderByCol != 0)
+			continue;
+		bool b;
+		struct Expr *expr = list->a[i].pExpr;
+		enum field_type type = sql_expr_type(expr);
+		if (type == FIELD_TYPE_ANY)
+			type = FIELD_TYPE_SCALAR;
+		uint32_t id;
+		struct coll *coll;
+		if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0)
+			return NULL;
+		info->types[k] = type;
+		info->coll_ids[k] = id;
+		++k;
+	}
+	assert(k == (int)field_count);
+	return info;
+}
+
+static struct sql_key_info *
+sql_key_info_new_from_space_info(const struct sql_space_info *info)
+{
+	uint32_t part_count = info->part_count - 1;
+	assert(part_count > 0);
+	uint32_t size = sizeof(struct sql_key_info) +
+			part_count * sizeof(struct key_part_def);
+	struct sql_key_info *key_info = sqlDbMallocRawNN(sql_get(), size);
+	if (key_info == NULL)
+		return NULL;
+	key_info->db = sql_get();
+	key_info->key_def = NULL;
+	key_info->refs = 1;
+	key_info->part_count = part_count;
+	for (uint32_t i = 0; i < part_count; ++i) {
+		key_info->parts[i].fieldno = i;
+		key_info->parts[i].nullable_action = ON_CONFLICT_ACTION_ABORT;
+		key_info->parts[i].is_nullable = false;
+		key_info->parts[i].exclude_null = false;
+		key_info->parts[i].sort_order = info->sort_orders[i];
+		key_info->parts[i].path = NULL;
+		key_info->parts[i].type = info->types[i];
+		key_info->parts[i].coll_id = info->coll_ids[i];
+	}
+	return key_info;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -819,13 +990,32 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 		if (pParse->db->mallocFailed)
 			return;
 		pOp->p2 = nKey + nData;
-		struct sql_key_info *key_info = pOp->p4.key_info;
+		assert(pOp->opcode == OP_OpenTEphemeral ||
+		       pOp->opcode == OP_SorterOpen);
+		struct sql_key_info *key_info =
+			pOp->opcode == OP_SorterOpen ? pOp->p4.key_info :
+			sql_key_info_new_from_space_info(pOp->p4.space_info);
 		for (uint32_t i = 0; i < key_info->part_count; i++)
 			key_info->parts[i].sort_order = SORT_ORDER_ASC;
 		sqlVdbeChangeP4(v, -1, (char *)key_info, P4_KEYINFO);
-		pOp->p4.key_info = sql_expr_list_to_key_info(pParse,
-							     pSort->pOrderBy,
-							     nOBSat);
+		if (pOp->opcode == OP_SorterOpen) {
+			pOp->p4.key_info =
+				sql_expr_list_to_key_info(pParse,
+							  pSort->pOrderBy,
+							  nOBSat);
+		} else {
+			struct sql_space_info *info =
+				sql_space_info_new_for_sorting(pParse,
+							       pSort->pOrderBy,
+							       pSelect->pEList,
+							       nOBSat, bSeq);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return;
+			}
+			sqlVdbeChangeP4(v, pSort->addrSortIndex, (char *)info,
+					P4_DYNAMIC);
+		}
 		addrJmp = sqlVdbeCurrentAddr(v);
 		sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
 		VdbeCoverage(v);
@@ -1044,21 +1234,33 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 			 * space format.
 			 */
 			uint32_t excess_field_count = 0;
+			struct VdbeOp *op = sqlVdbeGetOp(v,
+							 pSort->addrSortIndex);
 			for (i = pSort->nOBSat; i < pSort->pOrderBy->nExpr;
 			     i++) {
 				int j = pSort->pOrderBy->a[i].u.x.iOrderByCol;
-				if (j > 0) {
-					excess_field_count++;
-					pEList->a[j - 1].u.x.iOrderByCol =
-						(u16) (i + 1 - pSort->nOBSat);
+				if (j == 0)
+					continue;
+				assert(j > 0);
+				excess_field_count++;
+				pEList->a[j - 1].u.x.iOrderByCol =
+					(uint16_t)(i + 1 - pSort->nOBSat);
+				if (op->opcode != OP_OpenTEphemeral)
+					continue;
+				struct sql_space_info *info = op->p4.space_info;
+				--info->field_count;
+				for (int k = j; k < pEList->nExpr; ++k) {
+					int n = k + pSort->pOrderBy->nExpr + 1;
+					info->types[n - 1] = info->types[n];
+					info->coll_ids[n - 1] =
+						info->coll_ids[n];
 				}
 			}
-			struct VdbeOp *open_eph_op =
-				sqlVdbeGetOp(v, pSort->addrSortIndex);
-			assert(open_eph_op->p2 - excess_field_count > 0);
-			sqlVdbeChangeP2(v, pSort->addrSortIndex,
-					open_eph_op->p2 -
-					excess_field_count);
+			if (op->opcode != OP_OpenTEphemeral) {
+				assert(op->p2 - excess_field_count > 0);
+				sqlVdbeChangeP2(v, pSort->addrSortIndex,
+						op->p2 - excess_field_count);
+			}
 			regOrig = 0;
 			assert(eDest == SRT_Set || eDest == SRT_Mem
 			       || eDest == SRT_Coroutine
@@ -1419,7 +1621,6 @@ 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;
@@ -1434,24 +1635,6 @@ sql_key_info_new(sql *db, uint32_t part_count)
 	return key_info;
 }
 
-struct sql_key_info *
-sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def)
-{
-	struct sql_key_info *key_info = sqlDbMallocRawNN(db,
-				sql_key_info_sizeof(key_def->part_count));
-	if (key_info == NULL) {
-		sqlOomFault(db);
-		return NULL;
-	}
-	key_info->db = db;
-	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;
-}
-
 struct sql_key_info *
 sql_key_info_ref(struct sql_key_info *key_info)
 {
@@ -2453,22 +2636,26 @@ generateWithRecursiveQuery(Parse * pParse,	/* Parsing context */
 	/* Allocate cursors for Current, Queue, and Distinct. */
 	regCurrent = ++pParse->nMem;
 	sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol);
+	struct sql_space_info *info;
 	if (pOrderBy) {
-		struct sql_key_info *key_info =
-			sql_multiselect_orderby_to_key_info(pParse, p, 1);
-		sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue,
-				  pOrderBy->nExpr + 2, 0, (char *)key_info,
-				  P4_KEYINFO);
 		VdbeComment((v, "Orderby table"));
+		info = sql_space_info_new_from_order_by(pParse, p, pOrderBy);
 		destQueue.pOrderBy = pOrderBy;
 	} else {
-		sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1);
 		VdbeComment((v, "Queue table"));
+		info = sql_space_info_new_from_expr_list(pParse, p->pEList,
+							 true);
+	}
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto end_of_recursive_query;
 	}
+	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, 0, 0, (char *)info,
+		      P4_DYNAMIC);
 	sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
 	if (iDistinct) {
 		p->addrOpenEphm[0] =
-		    sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_dist, 1);
+			sqlVdbeAddOp1(v, OP_OpenTEphemeral, reg_dist);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, iDistinct, 0, reg_dist);
 		p->selFlags |= SF_UsesEphemeral;
 		VdbeComment((v, "Distinct table"));
@@ -2672,8 +2859,16 @@ multiSelect(Parse * pParse,	/* Parsing context */
 	 */
 	if (dest.eDest == SRT_EphemTab) {
 		assert(p->pEList);
-		int nCols = p->pEList->nExpr;
-		sqlVdbeAddOp2(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1);
+		struct sql_space_info *info =
+			sql_space_info_new_from_expr_list(pParse, p->pEList,
+							  true);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, 0, 0,
+			      (char *)info, P4_DYNAMIC);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
 		VdbeComment((v, "Destination temp"));
 		dest.eDest = SRT_Table;
@@ -2789,10 +2984,9 @@ multiSelect(Parse * pParse,	/* Parsing context */
 					unionTab = pParse->nTab++;
 					reg_union = ++pParse->nMem;
 					assert(p->pOrderBy == 0);
-					addr =
-					    sqlVdbeAddOp2(v,
-							      OP_OpenTEphemeral,
-							      reg_union, 0);
+					addr = sqlVdbeAddOp1(v,
+							     OP_OpenTEphemeral,
+							     reg_union);
 					sqlVdbeAddOp3(v, OP_IteratorOpen, unionTab, 0, reg_union);
 					assert(p->addrOpenEphm[0] == -1);
 					p->addrOpenEphm[0] = addr;
@@ -2905,9 +3099,8 @@ multiSelect(Parse * pParse,	/* Parsing context */
 				reg_eph2 = ++pParse->nMem;
 				assert(p->pOrderBy == 0);
 
-				addr =
-				    sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph1,
-						      0);
+				addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+						     reg_eph1);
 				sqlVdbeAddOp3(v, OP_IteratorOpen, tab1, 0, reg_eph1);
 				assert(p->addrOpenEphm[0] == -1);
 				p->addrOpenEphm[0] = addr;
@@ -2927,9 +3120,8 @@ multiSelect(Parse * pParse,	/* Parsing context */
 
 				/* Code the current SELECT into temporary table "tab2"
 				 */
-				addr =
-				    sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph2,
-						      0);
+				addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+						     reg_eph2);
 				sqlVdbeAddOp3(v, OP_IteratorOpen, tab2, 0, reg_eph2);
 				assert(p->addrOpenEphm[1] == -1);
 				p->addrOpenEphm[1] = addr;
@@ -3001,14 +3193,14 @@ multiSelect(Parse * pParse,	/* Parsing context */
 	if (p->selFlags & SF_UsesEphemeral) {
 		assert(p->pNext == NULL);
 		int nCol = p->pEList->nExpr;
-		struct sql_key_info *key_info = sql_key_info_new(db, nCol);
-		if (key_info == NULL)
+		struct sql_space_info *info = sql_space_info_new(nCol, 0);
+		if (info == NULL) {
+			pParse->is_aborted = true;
 			goto multi_select_end;
-		for (int i = 0; i < nCol; i++) {
-			key_info->parts[i].coll_id =
-				multi_select_coll_seq(pParse, p, i);
 		}
-
+		for (int i = 0; i < nCol; ++i)
+			info->coll_ids[i] = multi_select_coll_seq(pParse, p, i);
+		bool is_info_used = false;
 		for (struct Select *pLoop = p; pLoop; pLoop = pLoop->pPrior) {
 			for (int i = 0; i < 2; i++) {
 				int addr = pLoop->addrOpenEphm[i];
@@ -3020,13 +3212,15 @@ multiSelect(Parse * pParse,	/* Parsing context */
 					break;
 				}
 				sqlVdbeChangeP2(v, addr, nCol);
-				sqlVdbeChangeP4(v, addr,
-						    (char *)sql_key_info_ref(key_info),
-						    P4_KEYINFO);
+				sqlVdbeChangeP4(v, addr, (char *)info,
+						is_info_used ?
+						P4_STATIC : P4_DYNAMIC);
+				is_info_used = true;
 				pLoop->addrOpenEphm[i] = -1;
 			}
 		}
-		sql_key_info_unref(key_info);
+		if (!is_info_used)
+			sqlDbFree(pParse->db, info);
 	}
 
  multi_select_end:
@@ -5347,17 +5541,22 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo)
 					 "exactly one argument");
 				pParse->is_aborted = true;
 				pFunc->iDistinct = -1;
-			} else {
-				struct sql_key_info *key_info =
-					sql_expr_list_to_key_info(pParse,
+				return;
+			}
+			assert(pE->x.pList->nExpr == 1);
+			struct sql_space_info *info =
+				sql_space_info_new_from_expr_list(pParse,
 								  pE->x.pList,
-								  0);
-				sqlVdbeAddOp4(v, OP_OpenTEphemeral,
-						  pFunc->reg_eph, 1, 0,
-						  (char *)key_info, P4_KEYINFO);
-				sqlVdbeAddOp3(v, OP_IteratorOpen,
-						  pFunc->iDistinct, 0, pFunc->reg_eph);
+								  false);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
 			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 0,
+				      0, (char *)info, P4_DYNAMIC);
+			sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0,
+				      pFunc->reg_eph);
 		}
 	}
 }
@@ -5862,23 +6061,21 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	 * that change.
 	 */
 	if (sSort.pOrderBy) {
-		struct sql_key_info *key_info =
-			sql_expr_list_to_key_info(pParse, sSort.pOrderBy, 0);
 		sSort.reg_eph = ++pParse->nMem;
 		sSort.iECursor = pParse->nTab++;
-		/* Number of columns in transient table equals to number of columns in
-		 * SELECT statement plus number of columns in ORDER BY statement
-		 * and plus one column for ID.
-		 */
-		int nCols = pEList->nExpr + sSort.pOrderBy->nExpr + 1;
-		if (key_info->parts[0].sort_order == SORT_ORDER_DESC) {
-			sSort.sortFlags |= SORTFLAG_DESC;
+		struct sql_space_info *info =
+			sql_space_info_new_for_sorting(pParse, sSort.pOrderBy,
+						       pEList, 0, true);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
 		}
+		if (info->sort_orders[0] == SORT_ORDER_DESC)
+			sSort.sortFlags |= SORTFLAG_DESC;
 		sSort.addrSortIndex =
-		    sqlVdbeAddOp4(v, OP_OpenTEphemeral,
-				      sSort.reg_eph,
-				      nCols,
-				      0, (char *)key_info, P4_KEYINFO);
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, sSort.reg_eph, 0, 0,
+				      (char *)info, P4_DYNAMIC);
+
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
 	} else {
@@ -5888,11 +6085,14 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	/* If the output is destined for a temporary table, open that table.
 	 */
 	if (pDest->eDest == SRT_EphemTab) {
-		struct sql_key_info *key_info =
-			sql_expr_list_to_key_info(pParse, pEList, 0);
-		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph,
-				  pEList->nExpr + 1, 0, (char *)key_info,
-				  P4_KEYINFO);
+		struct sql_space_info *info =
+			sql_space_info_new_from_expr_list(pParse, pEList, true);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, 0, 0,
+			      (char *)info, P4_DYNAMIC);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
 				  pDest->reg_eph);
 
@@ -5907,8 +6107,15 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	}
 	computeLimitRegisters(pParse, p, iEnd);
 	if (p->iLimit == 0 && sSort.addrSortIndex >= 0) {
+		struct VdbeOp *op = sqlVdbeGetOp(v, sSort.addrSortIndex);
+		struct sql_key_info *key_info =
+			sql_key_info_new_from_space_info(op->p4.space_info);
 		sqlVdbeChangeOpcode(v, sSort.addrSortIndex, OP_SorterOpen);
 		sqlVdbeChangeP1(v, sSort.addrSortIndex, sSort.iECursor);
+		sqlVdbeChangeP2(v, sSort.addrSortIndex,
+				op->p4.space_info->field_count);
+		sqlVdbeChangeP4(v, sSort.addrSortIndex, (char *)key_info,
+				P4_KEYINFO);
 		sqlVdbeChangeToNoop(v, sSort.addrSortIndex + 1);
 		sSort.sortFlags |= SORTFLAG_UseSorter;
 	}
@@ -5918,13 +6125,16 @@ sqlSelect(Parse * pParse,		/* The parser context */
 	if (p->selFlags & SF_Distinct) {
 		sDistinct.cur_eph = pParse->nTab++;
 		sDistinct.reg_eph = ++pParse->nMem;
-		struct sql_key_info *key_info =
-			sql_expr_list_to_key_info(pParse, p->pEList, 0);
+		struct sql_space_info *info =
+			sql_space_info_new_from_expr_list(pParse, pEList,
+							  false);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		sDistinct.addrTnct = sqlVdbeAddOp4(v, OP_OpenTEphemeral,
-						       sDistinct.reg_eph,
-						       key_info->part_count,
-						       0, (char *)key_info,
-						       P4_KEYINFO);
+						   sDistinct.reg_eph, 0, 0,
+						   (char *)info, P4_DYNAMIC);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sDistinct.cur_eph, 0,
 				  sDistinct.reg_eph);
 		VdbeComment((v, "Distinct table"));
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1f87e6823..540c3a2ff 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4013,8 +4013,6 @@ 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. */
@@ -4028,12 +4026,6 @@ struct sql_key_info {
 struct sql_key_info *
 sql_key_info_new(sql *db, uint32_t part_count);
 
-/**
- * Allocate a key_info object from the given key definition.
- */
-struct sql_key_info *
-sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def);
-
 /**
  * Increment the reference counter of a key_info object.
  */
@@ -4055,6 +4047,64 @@ sql_key_info_unref(struct sql_key_info *key_info);
 struct key_def *
 sql_key_info_to_key_def(struct sql_key_info *key_info);
 
+/**
+ * Structure that is used to store information about ephemeral space field types
+ * and fieldno of key parts.
+ */
+struct sql_space_info {
+	/** Field types of all fields of ephemeral space. */
+	enum field_type *types;
+	/** Collation ids of all fields of ephemeral space. */
+	uint32_t *coll_ids;
+	/**
+	 * Fieldno key parts of the ephemeral space. If NULL, then the index
+	 * consists of all fields in sequential order.
+	 */
+	uint32_t *parts;
+	/** Sort order of index. */
+	enum sort_order *sort_orders;
+	/** Number of fields of ephemetal space. */
+	uint32_t field_count;
+	/**
+	 * Number of parts in primary index of ephemetal space. If 0 then parts
+	 * is also NULL.
+	 */
+	uint32_t part_count;
+};
+
+/**
+ * Allocate and initialize with default values a structure that will be used to
+ * store information about ephemeral space field types and key parts.
+ */
+struct sql_space_info *
+sql_space_info_new(uint32_t field_count, uint32_t part_count);
+
+/**
+ * Initialize the field types and key parts of space_info with space_def.
+ * Additionally added one more field type and key part for rowid. Rowid is
+ * always INTEGER. Key parts will be initialized with the same values as the
+ * field types. The number of initialized field types and key parts will be the
+ * same as the field_count in space_def plus one.
+ */
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def);
+
+/**
+ * Initialize the field types and key parts of space_info with index_def.
+ * Key parts will be initialized with the same values as the field types. The
+ * number of initialized field types and key parts will be the same as the
+ * part_count in index_def.
+ */
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid);
+
+/**
+ * Allocate and initialize an ephemeral space. Information about field types and
+ * key parts is taken from the space_info structure.
+ */
+struct space *
+sql_ephemeral_space_new(const struct sql_space_info *info);
+
 /**
  * Check if the function implements LIKE-style comparison & if it
  * is appropriate to apply a LIKE query optimization.
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 8fdc50432..adb7b5f9f 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -61,21 +61,6 @@ sql_rename_table(uint32_t space_id, const char *new_name);
 int tarantoolsqlRenameTrigger(const char *zTriggerName,
 				  const char *zOldName, const char *zNewName);
 
-/**
- * Create ephemeral space. Features of ephemeral spaces: id == 0,
- * name == "ephemeral", memtx engine (in future it can be changed,
- * but now only memtx engine is supported), primary index which
- * covers all fields and no secondary indexes, given field number
- * and collation sequence. All fields are scalar and nullable.
- *
- * @param field_count Number of fields in ephemeral space.
- * @param key_info Keys description for new ephemeral space.
- *
- * @retval Pointer to created space, NULL if error.
- */
-struct space *
-sql_ephemeral_space_create(uint32_t filed_count, struct sql_key_info *key_info);
-
 /**
  * Insert tuple into ephemeral space.
  * In contrast to ordinary spaces, there is no need to create and
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 22f82390c..b4827f242 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -226,9 +226,20 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	iEph = pParse->nTab++;
 	sqlVdbeAddOp2(v, OP_Null, 0, iPk);
 
+	struct sql_space_info *info;
+	assert(space->index_count > 0 || is_view);
+	if (is_view)
+		info = sql_space_info_new_from_space_def(def);
+	else
+		info = sql_space_info_new_from_index_def(pPk->def, false);
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto update_cleanup;
+	}
 	/* Address of the OpenEphemeral instruction. */
-	int addrOpen = sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
-					 pk_part_count);
+	int addrOpen = sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+				     (char *)info, P4_DYNAMIC);
+
 	pWInfo = sqlWhereBegin(pParse, pTabList, pWhere, 0, 0,
 				   WHERE_ONEPASS_DESIRED, pk_cursor);
 	if (pWInfo == 0)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index fcea9eefe..ce30afc5a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2033,10 +2033,9 @@ case OP_Fetch: {
 case OP_ApplyType: {
 	enum field_type *types = pOp->p4.types;
 	assert(types != NULL);
-	assert(types[pOp->p2] == field_type_MAX);
 	pIn1 = &aMem[pOp->p1];
-	enum field_type type;
-	while((type = *(types++)) != field_type_MAX) {
+	for (int i = 0; i < pOp->p2; ++i, ++pIn1) {
+		enum field_type type = types[i];
 		assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]);
 		assert(memIsValid(pIn1));
 		if (mem_cast_implicit(pIn1, type) != 0) {
@@ -2044,7 +2043,6 @@ case OP_ApplyType: {
 				 mem_str(pIn1), field_type_strs[type]);
 			goto abort_due_to_error;
 		}
-		pIn1++;
 	}
 	break;
 }
@@ -2380,10 +2378,9 @@ open_cursor_set_hints:
 }
 
 /**
- * Opcode: OpenTEphemeral P1 P2 * P4 *
+ * Opcode: OpenTEphemeral P1 * * P4 *
  * Synopsis:
  * @param P1 register, where pointer to new space is stored.
- * @param P2 number of columns in a new table.
  * @param P4 key def for new table, NULL is allowed.
  *
  * This opcode creates Tarantool's ephemeral table and stores pointer
@@ -2391,11 +2388,11 @@ open_cursor_set_hints:
  */
 case OP_OpenTEphemeral: {
 	assert(pOp->p1 >= 0);
-	assert(pOp->p2 > 0);
-	assert(pOp->p4type != P4_KEYINFO || pOp->p4.key_info != NULL);
 
-	struct space *space = sql_ephemeral_space_create(pOp->p2,
-							 pOp->p4.key_info);
+	assert(pOp->p4type == P4_DYNAMIC || pOp->p4type == P4_STATIC);
+	struct sql_space_info *info = pOp->p4.space_info;
+	assert(info != NULL);
+	struct space *space = sql_ephemeral_space_new(info);
 
 	if (space == NULL)
 		goto abort_due_to_error;
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index be112c72d..e40a1a0b3 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -93,6 +93,10 @@ struct VdbeOp {
 		 * doing a cast.
 		 */
 		enum field_type *types;
+		/**
+		 * Information about ephemeral space field types and key parts.
+		 */
+		struct sql_space_info *space_info;
 	} p4;
 #ifdef SQL_ENABLE_EXPLAIN_COMMENTS
 	char *zComment;		/* Comment to improve readability */
@@ -210,15 +214,6 @@ int sqlVdbeDeletePriorOpcode(Vdbe *, u8 op);
 void sqlVdbeChangeP4(Vdbe *, int addr, const char *zP4, int N);
 void sqlVdbeAppendP4(Vdbe *, void *pP4, int p4type);
 
-/**
- * Set the P4 on the most recently added opcode to the key_def for the
- * index given.
- * @param Parse context, for error reporting.
- * @param key_def Definition of a key to set.
- */
-void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def);
-
 VdbeOp *sqlVdbeGetOp(Vdbe *, int);
 int sqlVdbeMakeLabel(Vdbe *);
 void sqlVdbeRunOnlyOnce(Vdbe *);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 61be7b489..2d7800b17 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -787,18 +787,6 @@ sqlVdbeAppendP4(Vdbe * p, void *pP4, int n)
 	}
 }
 
-void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def)
-{
-	struct Vdbe *v = parse->pVdbe;
-	assert(v != NULL);
-	assert(key_def != NULL);
-	struct sql_key_info *key_info =
-		sql_key_info_new_from_key_def(parse->db, key_def);
-	if (key_info != NULL)
-		sqlVdbeAppendP4(v, key_info, P4_KEYINFO);
-}
-
 #ifdef SQL_ENABLE_EXPLAIN_COMMENTS
 /*
  * Change the comment on the most recently coded instruction.  Or
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 16766f2f8..d8d23161b 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,15 +919,15 @@ constructAutomaticIndex(Parse * pParse,			/* The parsing context */
 	/* Create the automatic index */
 	assert(pLevel->iIdxCur >= 0);
 	pLevel->iIdxCur = pParse->nTab++;
-	struct sql_key_info *pk_info =
-		sql_key_info_new_from_key_def(pParse->db, idx_def->key_def);
-	if (pk_info == NULL) {
+	struct sql_space_info *info = sql_space_info_new_from_index_def(idx_def,
+									true);
+	if (info == NULL) {
 		pParse->is_aborted = true;
 		return;
 	}
 	int reg_eph = sqlGetTempReg(pParse);
-	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nKeyCol + 1, 0,
-		      (char *)pk_info, P4_KEYINFO);
+	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+		      P4_DYNAMIC);
 	sqlVdbeAddOp3(v, OP_IteratorOpen, pLevel->iIdxCur, 0, reg_eph);
 	VdbeComment((v, "for %s", space->def->name));
 
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index df6cc92e1..0d0cb054d 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1134,11 +1134,19 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 		if ((pWInfo->wctrlFlags & WHERE_DUPLICATES_OK) == 0) {
 			cur_row_set = pParse->nTab++;
 			reg_row_set = ++pParse->nMem;
-			sqlVdbeAddOp2(v, OP_OpenTEphemeral,
-					  reg_row_set, pk_part_count);
+			struct index_def *index_def = space->index[0]->def;
+			struct sql_space_info *info =
+				sql_space_info_new_from_index_def(index_def,
+								  false);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return notReady;
+			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set,
+				      pk_part_count, 0, (char *)info,
+				      P4_DYNAMIC);
 			sqlVdbeAddOp3(v, OP_IteratorOpen, cur_row_set, 0,
 					  reg_row_set);
-			sql_vdbe_set_p4_key_def(pParse, pk_key_def);
 			regPk = ++pParse->nMem;
 		}
 		iRetInit = sqlVdbeAddOp2(v, OP_Integer, 0, regReturn);

  parent reply	other threads:[~2021-08-17 10:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17 10:43 [Tarantool-patches] [PATCH v2 0/2] Define field types in ephemeral spaces Mergen Imeev via Tarantool-patches
2021-08-17 10:43 ` [Tarantool-patches] [PATCH v2 1/2] sql: rename some sql_ephemeral_space_* functions Mergen Imeev via Tarantool-patches
2021-08-17 11:40   ` Vladimir Davydov via Tarantool-patches
2021-08-17 10:43 ` Mergen Imeev via Tarantool-patches [this message]
2021-08-17 11:54 [Tarantool-patches] [PATCH v2 0/2] Define field types in ephemeral spaces Mergen Imeev via Tarantool-patches
2021-08-17 11:55 ` [Tarantool-patches] [PATCH v2 2/2] sql: define ephemeral space fields Mergen Imeev via Tarantool-patches
2021-08-18 10:31   ` Vitaliia Ioffe via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9360d4e17f417b2286431159a4cd16714178fb5.1629196232.git.imeevma@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=vdavydov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 2/2] sql: define ephemeral space fields' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox