Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
@ 2021-07-15 14:14 Mergen Imeev via Tarantool-patches
  2021-07-23 23:06 ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-15 14:14 UTC (permalink / raw)
  To: v.shpilevoy, kyukhin; +Cc: tarantool-patches

After this patch, most of the created ephemeral spaces will have defined
field types in their format. Previously, for some fields, in some cases,
the type was defined, but mostly fields were of SCALAR type by default.

There are two cases where the old approach is used:
1) In the IN operator, since this operator accepts values of any scalar
types as valid right values.
2) In some cases during ORDER BY, as some optimizations use the old
approach. This case should be fixed in its own patch.

Part of #6213
---
https://github.com/tarantool/tarantool/issues/6213
https://github.com/tarantool/tarantool/tree/imeevma/gh-6213-field-types-in-ephemeral-spaces

 src/box/sql.c              | 119 ++++++++++++++++-
 src/box/sql/delete.c       |  41 +++---
 src/box/sql/expr.c         |  21 ++-
 src/box/sql/insert.c       |  49 ++++---
 src/box/sql/select.c       | 261 ++++++++++++++++++++++++++++++-------
 src/box/sql/sqlInt.h       |  25 ++++
 src/box/sql/tarantoolInt.h |  14 +-
 src/box/sql/update.c       |  20 ++-
 src/box/sql/vdbe.c         |  25 +++-
 src/box/sql/vdbe.h         |  15 +--
 src/box/sql/vdbeaux.c      |  15 +--
 src/box/sql/where.c        |  11 +-
 src/box/sql/wherecode.c    |  15 ++-
 13 files changed, 494 insertions(+), 137 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 790ca7f70..f5d0d6ce6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -298,7 +298,8 @@ tarantoolsqlCount(struct BtCursor *pCur)
 }
 
 struct space *
-sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
+sql_ephemeral_space_create_key_info(uint32_t field_count,
+				    struct sql_key_info *key_info)
 {
 	struct key_def *def = NULL;
 	uint32_t part_count = field_count;
@@ -407,6 +408,122 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
 	return ephemer_new_space;
 }
 
+enum {
+	/*
+	 * 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_LEN = 19,
+};
+
+static struct space_def *
+sql_space_def_new_ephemeral(uint32_t field_count, enum field_type *types,
+			    uint32_t *coll_ids)
+{
+	struct region *region = &fiber()->gc;
+	size_t svp = region_used(region);
+	uint32_t size = field_count * sizeof(struct field_def);
+	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;
+	}
+	char *names = region_alloc(region, field_count * NAME_LEN);
+	if (names == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "names");
+		return NULL;
+	}
+	for (uint32_t i = 0; i < field_count; ++i) {
+		struct field_def *field = &fields[i];
+		field->name = &names[i * 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;
+		field->type = types[i];
+		field->coll_id = coll_ids[i];
+	}
+	struct space_def *def = space_def_new_ephemeral(field_count, fields);
+	region_truncate(region, svp);
+	return def;
+}
+
+static struct index_def *
+sql_index_def_new_ephemeral(uint32_t field_count, enum field_type *types,
+			    uint32_t *coll_ids, bool is_pk_rowid)
+{
+	uint32_t part_count = is_pk_rowid ? 1 : field_count;
+	struct region *region = &fiber()->gc;
+	size_t svp = region_used(region);
+	uint32_t size = part_count * sizeof(struct key_part_def);
+	struct key_part_def *parts = region_aligned_alloc(region, size,
+							  alignof(parts[0]));
+	if (parts == NULL) {
+		diag_set(OutOfMemory, size, "region_aligned_alloc", "parts");
+		return NULL;
+	}
+	if (is_pk_rowid) {
+		uint32_t j = field_count - 1;
+		parts[0].fieldno = j;
+		parts[0].nullable_action = ON_CONFLICT_ACTION_NONE;
+		parts[0].is_nullable = true;
+		parts[0].exclude_null = false;
+		parts[0].sort_order = SORT_ORDER_ASC;
+		parts[0].path = NULL;
+		parts[0].type = types[j];
+		parts[0].coll_id = coll_ids[j];
+	} else {
+		for (uint32_t i = 0; i < part_count; ++i) {
+			struct key_part_def *part = &parts[i];
+			part->fieldno = i;
+			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 = types[i];
+			part->coll_id = coll_ids[i];
+		}
+	}
+	struct key_def *key_def = key_def_new(parts, part_count, false);
+	if (key_def == NULL)
+		return NULL;
+	const char *name = "ephemer_idx";
+	struct index_def *def = index_def_new(0, 0, name, strlen(name), TREE,
+					      &index_opts_default, key_def,
+					      NULL);
+	key_def_delete(key_def);
+	region_truncate(region, svp);
+	return def;
+}
+
+struct space *
+sql_ephemeral_space_create(uint32_t field_count, enum field_type *types,
+			   uint32_t *coll_ids, bool is_pk_rowid)
+{
+	struct space_def *space_def;
+	space_def = sql_space_def_new_ephemeral(field_count, types, coll_ids);
+	if (space_def == NULL)
+		return NULL;
+	struct index_def *index_def;
+	index_def = sql_index_def_new_ephemeral(field_count, types, coll_ids,
+						is_pk_rowid);
+	if (index_def == NULL) {
+		space_def_delete(space_def);
+		return NULL;
+	}
+	struct rlist key_list;
+	rlist_create(&key_list);
+	rlist_add_entry(&key_list, index_def, link);
+	struct space *space = space_new_ephemeral(space_def, &key_list);
+	index_def_delete(index_def);
+	space_def_delete(space_def);
+	return space;
+}
+
 int tarantoolsqlEphemeralInsert(struct space *space, const char *tuple,
 				    const char *tuple_end)
 {
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 62a726fdd..b35545296 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -224,12 +224,17 @@ 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 fields_info *info = fields_info_new(pk_len);
+		if (info == NULL) {
+			parse->is_aborted = true;
+			goto delete_from_cleanup;
+		}
 		if (is_view) {
 			/*
 			 * At this stage SELECT is already materialized
@@ -249,22 +254,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);
+			uint32_t count = space->def->field_count;
+			fields_info_from_space_def(info->types, info->coll_ids,
+						   count, space->def);
+			info->types[count] = FIELD_TYPE_UNSIGNED;
+			info->coll_ids[count] = COLL_NONE;
 		} 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);
+			assert(space->index_count > 0);
+			fields_info_from_index_def(info->types, info->coll_ids,
+						   pk_len,
+						   space->index[0]->def);
 		}
+		parse->nMem += pk_len;
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, pk_len, 0,
+			      (char *)info, P4_SPACEINFO);
 
 		/* Construct a query to find the primary key for
 		 * every row to be deleted, based on the WHERE
@@ -295,8 +298,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);
 			}
@@ -377,7 +381,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 3772596d6..cc7456371 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2773,7 +2773,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 */
 
@@ -2794,13 +2793,13 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 			 */
 			pExpr->iTable = pParse->nTab++;
 			int reg_eph = ++pParse->nMem;
-			addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral,
-						 reg_eph, nVal);
+			struct fields_info *info = fields_info_new(nVal);
+			if (info == NULL)
+				return 0;
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nVal, 0,
+				      (char *)info, P4_SPACEINFO);
 			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 ...)
@@ -2830,7 +2829,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,
@@ -2841,8 +2839,10 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 						Expr *p =
 						    sqlVectorFieldSubexpr
 						    (pLeft, i);
+						info->types[i] =
+							FIELD_TYPE_SCALAR;
 						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;
 					}
 				}
@@ -2863,8 +2863,9 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 					sql_expr_type(pLeft);
 				bool unused;
 				struct coll *unused_coll;
+				info->types[0] = FIELD_TYPE_SCALAR;
 				if (sql_expr_coll(pParse, pExpr->pLeft, &unused,
-						  &key_info->parts[0].coll_id,
+						  &info->coll_ids[0],
 						  &unused_coll) != 0)
 					return 0;
 
@@ -2899,8 +2900,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 02e9f9673..7d126f7e1 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -456,34 +456,41 @@ 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);
+			struct fields_info *info = fields_info_new(nColumn + 1);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				goto insert_cleanup;
+			}
+			if (pColumn != NULL) {
+				struct field_def *fields = space_def->fields;
+				for (int i = 0; i < nColumn; ++i) {
+					int j = pColumn->a[i].idx;
+					info->types[i] = fields[j].type;
+					info->coll_ids[i] = fields[j].coll_id;
+				}
+			} else {
+				fields_info_from_space_def(info->types,
+							   info->coll_ids,
+							   nColumn, space->def);
+			}
+			info->types[nColumn] = FIELD_TYPE_UNSIGNED;
+			info->coll_ids[nColumn] = COLL_NONE;
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
+				      nColumn + 1, 0, (char *)info,
+				      P4_SPACEINFO);
 			/*
-			 * 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.
 			 */
-			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);
-			}
+			sqlVdbeChangeP5(v, OPFLAG_ROWID_PK);
 			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 b9107fccc..c59d7b4b4 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -99,6 +99,103 @@ 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);
+
+struct fields_info *
+fields_info_new(uint32_t len)
+{
+	uint32_t size_info = sizeof(struct fields_info);
+	uint32_t size_types = len * sizeof(enum field_type);
+	uint32_t size = size_info + size_types + len * (sizeof(uint32_t));
+	char *buf = sqlDbMallocRawNN(sql_get(), size);
+	struct fields_info *info = (struct fields_info *)buf;
+	if (info == NULL)
+		return NULL;
+	info->key_info = NULL;
+	buf += size_info;
+	info->types = (enum field_type *)buf;
+	buf += size_types;
+	info->coll_ids = (uint32_t *)buf;
+	return info;
+}
+
+void
+fields_info_delete(struct fields_info *info)
+{
+	if (info->key_info != NULL)
+		sql_key_info_unref(info->key_info);
+	return sqlDbFree(sql_get(), info);
+}
+
+void
+fields_info_from_space_def(enum field_type *types, uint32_t *coll_ids,
+			   uint32_t count, struct space_def *def)
+{
+	for (uint32_t i = 0; i < count; ++i) {
+		types[i] = def->fields[i].type;
+		coll_ids[i] = def->fields[i].coll_id;
+	}
+}
+
+void
+fields_info_from_index_def(enum field_type *types, uint32_t *coll_ids,
+			   uint32_t count, struct index_def *def)
+{
+	for (uint32_t i = 0; i < count; ++i) {
+		types[i] = def->key_def->parts[i].type;
+		coll_ids[i] = def->key_def->parts[i].coll_id;
+	}
+}
+
+static int
+fields_info_from_expr_list(enum field_type *types, uint32_t *coll_ids,
+			   struct Parse *parser, struct ExprList *list)
+{
+	for (int i = 0; i < list->nExpr; ++i) {
+		bool b;
+		struct coll *coll;
+		struct Expr *expr = list->a[i].pExpr;
+		types[i] = sql_expr_type(expr);
+		if (types[i] == FIELD_TYPE_ANY)
+			types[i] = FIELD_TYPE_SCALAR;
+		if (sql_expr_coll(parser, expr, &b, &coll_ids[i], &coll) != 0)
+			return -1;
+	}
+	return 0;
+}
+
+static int
+fields_info_from_order_by(enum field_type *types, uint32_t *coll_ids,
+			  struct Parse *parser, struct Select *select,
+			  struct ExprList *order_by)
+{
+	for (int i = 0; i < order_by->nExpr; ++i) {
+		bool b;
+		struct Expr *expr = order_by->a[i].pExpr;
+		types[i] = sql_expr_type(expr);
+		if (types[i] == FIELD_TYPE_ANY)
+			types[i] = FIELD_TYPE_SCALAR;
+		uint32_t id = COLL_NONE;
+		if ((expr->flags & EP_Collate) != 0) {
+			struct coll *coll;
+			if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0)
+				return -1;
+		} else {
+			uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
+			id = multi_select_coll_seq(parser, select, fieldno);
+			if (id != COLL_NONE) {
+				const char *name = coll_by_id(id)->name;
+				order_by->a[i].pExpr =
+					sqlExprAddCollateString(parser, expr,
+								name);
+			}
+		}
+		coll_ids[i] = id;
+	}
+	return 0;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -819,13 +916,13 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 		if (pParse->db->mallocFailed)
 			return;
 		pOp->p2 = nKey + nData;
-		struct sql_key_info *key_info = pOp->p4.key_info;
+		struct sql_key_info *key_info = pOp->p4.fields->key_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);
+		pOp->p4.fields->key_info =
+			sql_expr_list_to_key_info(pParse, pSort->pOrderBy,
+						  nOBSat);
 		addrJmp = sqlVdbeCurrentAddr(v);
 		sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
 		VdbeCoverage(v);
@@ -2453,16 +2550,35 @@ generateWithRecursiveQuery(Parse * pParse,	/* Parsing context */
 	/* Allocate cursors for Current, Queue, and Distinct. */
 	regCurrent = ++pParse->nMem;
 	sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol);
-	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);
+
+	int field_count = pOrderBy != NULL ? pOrderBy->nExpr + 2 : nCol + 1;
+	struct fields_info *info = fields_info_new(field_count);
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto end_of_recursive_query;
+	}
+	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, field_count, 0,
+		      (char *)info, P4_SPACEINFO);
+	if (pOrderBy != NULL) {
+		if (fields_info_from_order_by(info->types, info->coll_ids,
+					      pParse, p, pOrderBy) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
+		info->types[pOrderBy->nExpr] = FIELD_TYPE_UNSIGNED;
+		info->coll_ids[pOrderBy->nExpr] = COLL_NONE;
+		info->types[pOrderBy->nExpr + 1] = FIELD_TYPE_VARBINARY;
+		info->coll_ids[pOrderBy->nExpr + 1] = COLL_NONE;
 		VdbeComment((v, "Orderby table"));
 		destQueue.pOrderBy = pOrderBy;
 	} else {
-		sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1);
+		if (fields_info_from_expr_list(info->types, info->coll_ids,
+					       pParse, p->pEList) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
+		info->types[nCol] = FIELD_TYPE_UNSIGNED;
+		info->coll_ids[nCol] = COLL_NONE;
 		VdbeComment((v, "Queue table"));
 	}
 	sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
@@ -2673,7 +2789,23 @@ 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 fields_info *info = fields_info_new(nCols + 1);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		if (fields_info_from_expr_list(info->types, info->coll_ids,
+					       pParse, p->pEList) != 0) {
+			fields_info_delete(info);
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		info->types[nCols] = FIELD_TYPE_SCALAR;
+		info->coll_ids[nCols] = COLL_NONE;
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1,
+			      0, (char *)info, P4_SPACEINFO);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
 		VdbeComment((v, "Destination temp"));
 		dest.eDest = SRT_Table;
@@ -3001,14 +3133,17 @@ 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 fields_info *info = fields_info_new(nCol);
+		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->types[i] = FIELD_TYPE_SCALAR;
+			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 +3155,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_SPACEINFO);
+				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 +5484,27 @@ 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,
-								  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);
+				return;
+			}
+			assert(pE->x.pList->nExpr == 1);
+			struct fields_info *info = fields_info_new(1);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
+			}
+			if (fields_info_from_expr_list(info->types,
+						       info->coll_ids,
+						       pParse,
+						       pE->x.pList) != 0) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
 			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1,
+				      0, (char *)info, P4_SPACEINFO);
+			sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0,
+				      pFunc->reg_eph);
 		}
 	}
 }
@@ -5874,11 +6021,15 @@ sqlSelect(Parse * pParse,		/* The parser context */
 		if (key_info->parts[0].sort_order == SORT_ORDER_DESC) {
 			sSort.sortFlags |= SORTFLAG_DESC;
 		}
-		sSort.addrSortIndex =
-		    sqlVdbeAddOp4(v, OP_OpenTEphemeral,
-				      sSort.reg_eph,
-				      nCols,
-				      0, (char *)key_info, P4_KEYINFO);
+		struct fields_info *info = fields_info_new(nCols);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		info->key_info = key_info;
+		sSort.addrSortIndex = sqlVdbeAddOp4(v, OP_OpenTEphemeral,
+						    sSort.reg_eph, nCols, 0,
+						    (char *)info, P4_SPACEINFO);
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
 	} else {
@@ -5888,11 +6039,20 @@ 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);
+		struct fields_info *info = fields_info_new(pEList->nExpr + 1);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph,
-				  pEList->nExpr + 1, 0, (char *)key_info,
-				  P4_KEYINFO);
+			      pEList->nExpr + 1, 0, (char *)info, P4_SPACEINFO);
+		if (fields_info_from_expr_list(info->types, info->coll_ids,
+					       pParse, pEList) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		info->types[pEList->nExpr] = FIELD_TYPE_UNSIGNED;
+		info->coll_ids[pEList->nExpr] = COLL_NONE;
 		sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
 				  pDest->reg_eph);
 
@@ -5918,16 +6078,23 @@ 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 fields_info *info = fields_info_new(pEList->nExpr);
+		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,
+						   pEList->nExpr, 0,
+						   (char *)info, P4_SPACEINFO);
+		VdbeComment((v, "Distinct table"));
+		if (fields_info_from_expr_list(info->types, info->coll_ids,
+					       pParse, pEList) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sDistinct.cur_eph, 0,
 				  sDistinct.reg_eph);
-		VdbeComment((v, "Distinct table"));
 		sDistinct.eTnctType = WHERE_DISTINCT_UNORDERED;
 	} else {
 		sDistinct.eTnctType = WHERE_DISTINCT_NOOP;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index ef8dcd693..167e80057 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2248,6 +2248,11 @@ struct Parse {
 #define OPFLAG_SYSTEMSP      0x20	/* OP_Open**: set if space pointer
 					 * points to system space.
 					 */
+/**
+ * If this flag is set, than in OP_OpenTEphemeral we should use rowid as the
+ * only part of primary index.
+ */
+#define OPFLAG_ROWID_PK		0x01
 
 /**
  * Prepare vdbe P5 flags for OP_{IdxInsert, IdxReplace, Update}
@@ -4073,6 +4078,26 @@ 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);
 
+struct fields_info {
+	struct sql_key_info *key_info;
+	enum field_type *types;
+	uint32_t *coll_ids;
+};
+
+struct fields_info *
+fields_info_new(uint32_t len);
+
+void
+fields_info_delete(struct fields_info *info);
+
+void
+fields_info_from_space_def(enum field_type *types, uint32_t *coll_ids,
+			   uint32_t count, struct space_def *def);
+
+void
+fields_info_from_index_def(enum field_type *types, uint32_t *coll_ids,
+			   uint32_t count, struct index_def *def);
+
 /**
  * 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 1ded6c709..a85c406d8 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -71,7 +71,19 @@ int tarantoolsqlRenameTrigger(const char *zTriggerName,
  * @retval Pointer to created space, NULL if error.
  */
 struct space *
-sql_ephemeral_space_create(uint32_t filed_count, struct sql_key_info *key_info);
+sql_ephemeral_space_create_key_info(uint32_t field_count,
+				    struct sql_key_info *key_info);
+
+/**
+ * Create an ephemeral space. The number of fields, their types and collations
+ * of the new ephemeral space will be determined using the given arguments. In
+ * case is_pk_rowid is TRUE, the primary index will contain only one field -
+ * rowid (which is assumed to be the last one). If it is FALSE, then the primary
+ * index must contain all fields.
+ */
+struct space *
+sql_ephemeral_space_create(uint32_t field_count, enum field_type *types,
+			   uint32_t *coll_ids, bool is_pk_rowid);
 
 /**
  * Insert tuple into ephemeral space.
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 24c7cfa27..81552f5f7 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -227,8 +227,24 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	sqlVdbeAddOp2(v, OP_Null, 0, iPk);
 
 	/* Address of the OpenEphemeral instruction. */
-	int addrOpen = sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
-					 pk_part_count);
+	struct fields_info *info = fields_info_new(pk_part_count);
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto update_cleanup;
+	}
+	if (is_view) {
+		uint32_t n = space->def->field_count;
+		fields_info_from_space_def(info->types, info->coll_ids, n, def);
+		info->types[n] = FIELD_TYPE_UNSIGNED;
+		info->coll_ids[n] = COLL_NONE;
+	} else {
+		assert(space->index_count > 0);
+		fields_info_from_index_def(info->types, info->coll_ids,
+					   pk_part_count, pPk->def);
+	}
+	int addrOpen = sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
+				     pk_part_count, 0, (char *)info,
+				     P4_SPACEINFO);
 	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 cc698b715..03138e2c3 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2145,10 +2145,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) {
+		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) {
@@ -2521,10 +2520,18 @@ 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);
+	assert(pOp->p4type == P4_SPACEINFO || pOp->p4type == P4_STATIC);
 
-	struct space *space = sql_ephemeral_space_create(pOp->p2,
-							 pOp->p4.key_info);
+	struct space *space;
+	if (pOp->p4.fields->key_info == NULL) {
+		enum field_type *types = pOp->p4.fields->types;
+		uint32_t *coll_ids = pOp->p4.fields->coll_ids;
+		space = sql_ephemeral_space_create(pOp->p2, types, coll_ids,
+						   pOp->p5 == OPFLAG_ROWID_PK);
+	} else {
+		struct sql_key_info *key_info = pOp->p4.fields->key_info;
+		space = sql_ephemeral_space_create_key_info(pOp->p2, key_info);
+	}
 
 	if (space == NULL)
 		goto abort_due_to_error;
@@ -2547,7 +2554,11 @@ case OP_SorterOpen: {
 
 	assert(pOp->p1>=0);
 	assert(pOp->p2>=0);
-	struct key_def *def = sql_key_info_to_key_def(pOp->p4.key_info);
+	struct key_def *def;
+	if (pOp->p4type == P4_SPACEINFO)
+		def = sql_key_info_to_key_def(pOp->p4.fields->key_info);
+	else
+		def = sql_key_info_to_key_def(pOp->p4.key_info);
 	if (def == NULL) goto no_mem;
 	pCx = allocateCursor(p, pOp->p1, pOp->p2, CURTYPE_SORTER);
 	if (pCx==0) goto no_mem;
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 118f1cd83..a9d1d6a5e 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -93,6 +93,7 @@ struct VdbeOp {
 		 * doing a cast.
 		 */
 		enum field_type *types;
+		struct fields_info *fields;
 	} p4;
 #ifdef SQL_ENABLE_EXPLAIN_COMMENTS
 	char *zComment;		/* Comment to improve readability */
@@ -142,6 +143,11 @@ struct SubProgram {
 #define P4_PTR      (-18)	/* P4 is a generic pointer */
 #define P4_KEYINFO  (-19)       /* P4 is a pointer to sql_key_info structure. */
 #define P4_SPACEPTR (-20)       /* P4 is a space pointer */
+/**
+ * P4 is a structure that contains information about types and collations of
+ * fields.
+ */
+#define P4_SPACEINFO (-21)
 
 /* Error message codes for OP_Halt */
 #define P5_ConstraintNotNull 1
@@ -210,15 +216,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 4a1fdb637..02a3ed878 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -613,6 +613,9 @@ freeP4(sql * db, int p4type, void *p4)
 			sqlDbFree(db, p4);
 			break;
 		}
+	case P4_SPACEINFO:
+		fields_info_delete(p4);
+		break;
 	case P4_KEYINFO:
 		sql_key_info_unref(p4);
 		break;
@@ -787,18 +790,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 e5f35fbf8..79c4a5b82 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,15 +919,18 @@ 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 fields_info *info = fields_info_new(nKeyCol + 1);
+	if (info == NULL) {
 		pParse->is_aborted = true;
 		return;
 	}
+	fields_info_from_index_def(info->types, info->coll_ids, nKeyCol,
+				   idx_def);
+	info->types[nKeyCol] = FIELD_TYPE_UNSIGNED;
+	info->coll_ids[nKeyCol] = COLL_NONE;
 	int reg_eph = sqlGetTempReg(pParse);
 	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nKeyCol + 1, 0,
-		      (char *)pk_info, P4_KEYINFO);
+		      (char *)info, P4_SPACEINFO);
 	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 96bcab110..5b689cfff 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1345,11 +1345,20 @@ 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 fields_info *info =
+				fields_info_new(pk_part_count);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return notReady;
+			}
+			fields_info_from_index_def(info->types, info->coll_ids,
+						   pk_part_count,
+						   space_index(space, 0)->def);
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set,
+				      pk_part_count, 0, (char *)info,
+				      P4_SPACEINFO);
 			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);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
  2021-07-15 14:14 [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces Mergen Imeev via Tarantool-patches
@ 2021-07-23 23:06 ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-10 14:32   ` Mergen Imeev via Tarantool-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-23 23:06 UTC (permalink / raw)
  To: imeevma, kyukhin; +Cc: tarantool-patches

Hi! Thanks for the patch!

See 9 comments below.

> diff --git a/src/box/sql.c b/src/box/sql.c
> index 790ca7f70..f5d0d6ce6 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -298,7 +298,8 @@ tarantoolsqlCount(struct BtCursor *pCur)
>  }
>  
>  struct space *
> -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> +sql_ephemeral_space_create_key_info(uint32_t field_count,
> +				    struct sql_key_info *key_info)

1. The function creates not key_info. It creates a space by key_info.
Maybe call it sql_ephemeral_space_new_from_key_info()?

>  {
>  	struct key_def *def = NULL;
>  	uint32_t part_count = field_count;
> @@ -407,6 +408,122 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>  	return ephemer_new_space;
>  }
>  
> +enum {
> +	/*
> +	 * 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_LEN = 19,
> +};
> +
> +static struct space_def *
> +sql_space_def_new_ephemeral(uint32_t field_count, enum field_type *types,
> +			    uint32_t *coll_ids)

2. The last 2 arguments can be made const. The same in the other functions
below.

> +{
> +	struct region *region = &fiber()->gc;
> +	size_t svp = region_used(region);
> +	uint32_t size = field_count * sizeof(struct field_def);
> +	struct field_def *fields = region_aligned_alloc(region, size,
> +							alignof(fields[0]));

3. There is region_alloc_array(). The same in the next function.

> +	if (fields == NULL) {
> +		diag_set(OutOfMemory, size, "region_aligned_alloc", "fields");
> +		return NULL;
> +	}
> +	char *names = region_alloc(region, field_count * NAME_LEN);
> +	if (names == NULL) {
> +		diag_set(OutOfMemory, size, "region_alloc", "names");> +		return NULL;
> +	}
> +	for (uint32_t i = 0; i < field_count; ++i) {
> +		struct field_def *field = &fields[i];
> +		field->name = &names[i * 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;
> +		field->type = types[i];
> +		field->coll_id = coll_ids[i];
> +	}
> +	struct space_def *def = space_def_new_ephemeral(field_count, fields);
> +	region_truncate(region, svp);
> +	return def;
> +}
> +
> +static struct index_def *
> +sql_index_def_new_ephemeral(uint32_t field_count, enum field_type *types,
> +			    uint32_t *coll_ids, bool is_pk_rowid)
> +{
> +	uint32_t part_count = is_pk_rowid ? 1 : field_count;
> +	struct region *region = &fiber()->gc;
> +	size_t svp = region_used(region);
> +	uint32_t size = part_count * sizeof(struct key_part_def);
> +	struct key_part_def *parts = region_aligned_alloc(region, size,
> +							  alignof(parts[0]));
> +	if (parts == NULL) {
> +		diag_set(OutOfMemory, size, "region_aligned_alloc", "parts");
> +		return NULL;
> +	}
> +	if (is_pk_rowid) {
> +		uint32_t j = field_count - 1;
> +		parts[0].fieldno = j;
> +		parts[0].nullable_action = ON_CONFLICT_ACTION_NONE;
> +		parts[0].is_nullable = true;
> +		parts[0].exclude_null = false;
> +		parts[0].sort_order = SORT_ORDER_ASC;
> +		parts[0].path = NULL;
> +		parts[0].type = types[j];
> +		parts[0].coll_id = coll_ids[j];
> +	} else {
> +		for (uint32_t i = 0; i < part_count; ++i) {
> +			struct key_part_def *part = &parts[i];
> +			part->fieldno = i;
> +			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 = types[i];
> +			part->coll_id = coll_ids[i];
> +		}
> +	}
> +	struct key_def *key_def = key_def_new(parts, part_count, false);
> +	if (key_def == NULL)
> +		return NULL;
> +	const char *name = "ephemer_idx";
> +	struct index_def *def = index_def_new(0, 0, name, strlen(name), TREE,
> +					      &index_opts_default, key_def,
> +					      NULL);
> +	key_def_delete(key_def);
> +	region_truncate(region, svp);
> +	return def;
> +}
> +
> +struct space *
> +sql_ephemeral_space_create(uint32_t field_count, enum field_type *types,
> +			   uint32_t *coll_ids, bool is_pk_rowid)

4. It is strange how there are functions

	sql_space_def_new_ephemeral
	sql_index_def_new_ephemeral

and at the same time these:

	sql_ephemeral_space_create_key_info
	sql_ephemeral_space_create

First ones use ephemeral in the end, the last ones in the beginning.
I would propose to unify them while you are here. One of the
options:

	sql_ephemeral_space_def_new
	sql_ephemeral_index_def_new
	sql_ephemeral_space_new_from_key_info
	sql_ephemeral_space_new

> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index 62a726fdd..b35545296 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -249,22 +254,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);
> +			uint32_t count = space->def->field_count;
> +			fields_info_from_space_def(info->types, info->coll_ids,
> +						   count, space->def);

5. In all usages of fields_info_from_space_def you pass 2 fields of
field_info. I propose to make it take field_info pointer instead
of these 2 arrays. Also for individual fields I propose to add
field_info_set_field(type, coll_id). So as not to miss anything. Currently
it looks too much boilerplate code.

> +			info->types[count] = FIELD_TYPE_UNSIGNED;
> +			info->coll_ids[count] = COLL_NONE;
>  		} else {
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index b9107fccc..c59d7b4b4 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -99,6 +99,103 @@ 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);
> +
> +struct fields_info *
> +fields_info_new(uint32_t len)
> +{
> +	uint32_t size_info = sizeof(struct fields_info);
> +	uint32_t size_types = len * sizeof(enum field_type);
> +	uint32_t size = size_info + size_types + len * (sizeof(uint32_t));
> +	char *buf = sqlDbMallocRawNN(sql_get(), size);
> +	struct fields_info *info = (struct fields_info *)buf;
> +	if (info == NULL)
> +		return NULL;
> +	info->key_info = NULL;
> +	buf += size_info;
> +	info->types = (enum field_type *)buf;
> +	buf += size_types;
> +	info->coll_ids = (uint32_t *)buf;
> +	return info;
> +}
> +
> +void
> +fields_info_delete(struct fields_info *info)
> +{
> +	if (info->key_info != NULL)
> +		sql_key_info_unref(info->key_info);
> +	return sqlDbFree(sql_get(), info);

6. You do not need this explicit 'return'.

> +}
> +
> +void
> +fields_info_from_space_def(enum field_type *types, uint32_t *coll_ids,
> +			   uint32_t count, struct space_def *def)

7. def can be made const. The same for some other arguments below.

> +{
> +	for (uint32_t i = 0; i < count; ++i) {
> +		types[i] = def->fields[i].type;
> +		coll_ids[i] = def->fields[i].coll_id;
> +	}
> +}
> +
> +void
> +fields_info_from_index_def(enum field_type *types, uint32_t *coll_ids,
> +			   uint32_t count, struct index_def *def)
> +{
> +	for (uint32_t i = 0; i < count; ++i) {
> +		types[i] = def->key_def->parts[i].type;
> +		coll_ids[i] = def->key_def->parts[i].coll_id;
> +	}
> +}
> +
> +static int
> +fields_info_from_expr_list(enum field_type *types, uint32_t *coll_ids,
> +			   struct Parse *parser, struct ExprList *list)
> +{
> +	for (int i = 0; i < list->nExpr; ++i) {
> +		bool b;
> +		struct coll *coll;
> +		struct Expr *expr = list->a[i].pExpr;
> +		types[i] = sql_expr_type(expr);
> +		if (types[i] == FIELD_TYPE_ANY)
> +			types[i] = FIELD_TYPE_SCALAR;
> +		if (sql_expr_coll(parser, expr, &b, &coll_ids[i], &coll) != 0)
> +			return -1;
> +	}
> +	return 0;
> +}
> +
> +static int
> +fields_info_from_order_by(enum field_type *types, uint32_t *coll_ids,
> +			  struct Parse *parser, struct Select *select,
> +			  struct ExprList *order_by)
> +{
> +	for (int i = 0; i < order_by->nExpr; ++i) {
> +		bool b;
> +		struct Expr *expr = order_by->a[i].pExpr;
> +		types[i] = sql_expr_type(expr);
> +		if (types[i] == FIELD_TYPE_ANY)
> +			types[i] = FIELD_TYPE_SCALAR;
> +		uint32_t id = COLL_NONE;
> +		if ((expr->flags & EP_Collate) != 0) {
> +			struct coll *coll;
> +			if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0)
> +				return -1;
> +		} else {
> +			uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
> +			id = multi_select_coll_seq(parser, select, fieldno);
> +			if (id != COLL_NONE) {
> +				const char *name = coll_by_id(id)->name;
> +				order_by->a[i].pExpr =
> +					sqlExprAddCollateString(parser, expr,
> +								name);
> +			}
> +		}
> +		coll_ids[i] = id;
> +	}
> +	return 0;
> +}

8. It sligtly cuts the ears that a struct called fields_info also
defines a key via key_info. It is like specifying indexes inside of
space format.

Maybe it is worth renaming it to ephemeral_space_def? Or space_info?
Or shorten ephemeral to ephmen in all the new code and use
ephem_space_def? Or sql_space_info? sql_ephemeral_space_def?

And make sql_ephemeral_space_create() take this struct instead of
2 arrays. Besides, you could hide sql_ephemeral_space_create_key_info()
inside of it because you could check key_info != null in
sql_ephemeral_space_create(). It just does not seem like vdbe business
to check these things.

> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index ef8dcd693..167e80057 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -2248,6 +2248,11 @@ struct Parse {
>  #define OPFLAG_SYSTEMSP      0x20	/* OP_Open**: set if space pointer
>  					 * points to system space.
>  					 */
> +/**
> + * If this flag is set, than in OP_OpenTEphemeral we should use rowid as the

9. than -> then.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
  2021-07-23 23:06 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-08-10 14:32   ` Mergen Imeev via Tarantool-patches
  2021-08-12 18:32     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-10 14:32 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thank you for the review! My answers and new patch below. Also, new patch
is a bit bigger than the old one since I could't think of proper reason do
divide it into more than one patch.

On Sat, Jul 24, 2021 at 01:06:30AM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 9 comments below.
> 
> > diff --git a/src/box/sql.c b/src/box/sql.c
> > index 790ca7f70..f5d0d6ce6 100644
> > --- a/src/box/sql.c
> > +++ b/src/box/sql.c
> > @@ -298,7 +298,8 @@ tarantoolsqlCount(struct BtCursor *pCur)
> >  }
> >  
> >  struct space *
> > -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> > +sql_ephemeral_space_create_key_info(uint32_t field_count,
> > +				    struct sql_key_info *key_info)
> 
> 1. The function creates not key_info. It creates a space by key_info.
> Maybe call it sql_ephemeral_space_new_from_key_info()?
> 
I dropped old function. A new function is now named
sql_ephemeral_space_new_from_info().

> >  {
> >  	struct key_def *def = NULL;
> >  	uint32_t part_count = field_count;
> > @@ -407,6 +408,122 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> >  	return ephemer_new_space;
> >  }
> >  
> > +enum {
> > +	/*
> > +	 * 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_LEN = 19,
> > +};
> > +
> > +static struct space_def *
> > +sql_space_def_new_ephemeral(uint32_t field_count, enum field_type *types,
> > +			    uint32_t *coll_ids)
> 
> 2. The last 2 arguments can be made const. The same in the other functions
> below.
> 
Fixed for new function.

> > +{
> > +	struct region *region = &fiber()->gc;
> > +	size_t svp = region_used(region);
> > +	uint32_t size = field_count * sizeof(struct field_def);
> > +	struct field_def *fields = region_aligned_alloc(region, size,
> > +							alignof(fields[0]));
> 
> 3. There is region_alloc_array(). The same in the next function.
> 
I am not sure tha it is proper to use this function here since the memory
alloced here will contain field_def, key_part_def and names of fields.

> > +	if (fields == NULL) {
> > +		diag_set(OutOfMemory, size, "region_aligned_alloc", "fields");
> > +		return NULL;
> > +	}
> > +	char *names = region_alloc(region, field_count * NAME_LEN);
> > +	if (names == NULL) {
> > +		diag_set(OutOfMemory, size, "region_alloc", "names");> +		return NULL;
> > +	}
> > +	for (uint32_t i = 0; i < field_count; ++i) {
> > +		struct field_def *field = &fields[i];
> > +		field->name = &names[i * 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;
> > +		field->type = types[i];
> > +		field->coll_id = coll_ids[i];
> > +	}
> > +	struct space_def *def = space_def_new_ephemeral(field_count, fields);
> > +	region_truncate(region, svp);
> > +	return def;
> > +}
> > +
> > +static struct index_def *
> > +sql_index_def_new_ephemeral(uint32_t field_count, enum field_type *types,
> > +			    uint32_t *coll_ids, bool is_pk_rowid)
> > +{
> > +	uint32_t part_count = is_pk_rowid ? 1 : field_count;
> > +	struct region *region = &fiber()->gc;
> > +	size_t svp = region_used(region);
> > +	uint32_t size = part_count * sizeof(struct key_part_def);
> > +	struct key_part_def *parts = region_aligned_alloc(region, size,
> > +							  alignof(parts[0]));
> > +	if (parts == NULL) {
> > +		diag_set(OutOfMemory, size, "region_aligned_alloc", "parts");
> > +		return NULL;
> > +	}
> > +	if (is_pk_rowid) {
> > +		uint32_t j = field_count - 1;
> > +		parts[0].fieldno = j;
> > +		parts[0].nullable_action = ON_CONFLICT_ACTION_NONE;
> > +		parts[0].is_nullable = true;
> > +		parts[0].exclude_null = false;
> > +		parts[0].sort_order = SORT_ORDER_ASC;
> > +		parts[0].path = NULL;
> > +		parts[0].type = types[j];
> > +		parts[0].coll_id = coll_ids[j];
> > +	} else {
> > +		for (uint32_t i = 0; i < part_count; ++i) {
> > +			struct key_part_def *part = &parts[i];
> > +			part->fieldno = i;
> > +			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 = types[i];
> > +			part->coll_id = coll_ids[i];
> > +		}
> > +	}
> > +	struct key_def *key_def = key_def_new(parts, part_count, false);
> > +	if (key_def == NULL)
> > +		return NULL;
> > +	const char *name = "ephemer_idx";
> > +	struct index_def *def = index_def_new(0, 0, name, strlen(name), TREE,
> > +					      &index_opts_default, key_def,
> > +					      NULL);
> > +	key_def_delete(key_def);
> > +	region_truncate(region, svp);
> > +	return def;
> > +}
> > +
> > +struct space *
> > +sql_ephemeral_space_create(uint32_t field_count, enum field_type *types,
> > +			   uint32_t *coll_ids, bool is_pk_rowid)
> 
> 4. It is strange how there are functions
> 
> 	sql_space_def_new_ephemeral
> 	sql_index_def_new_ephemeral
> 
> and at the same time these:
> 
> 	sql_ephemeral_space_create_key_info
> 	sql_ephemeral_space_create
> 
> First ones use ephemeral in the end, the last ones in the beginning.
> I would propose to unify them while you are here. One of the
> options:
> 
> 	sql_ephemeral_space_def_new
> 	sql_ephemeral_index_def_new
> 	sql_ephemeral_space_new_from_key_info
> 	sql_ephemeral_space_new
> 
Thank you for suggestion, but I dropped most of these function. New functions:
sql_ephemeral_space_new_from_info
sql_space_info_from_space_def
sql_space_info_from_index_def

And couple of static functions:
space_info_from_expr_list
space_info_from_order_by

> > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> > index 62a726fdd..b35545296 100644
> > --- a/src/box/sql/delete.c
> > +++ b/src/box/sql/delete.c
> > @@ -249,22 +254,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);
> > +			uint32_t count = space->def->field_count;
> > +			fields_info_from_space_def(info->types, info->coll_ids,
> > +						   count, space->def);
> 
> 5. In all usages of fields_info_from_space_def you pass 2 fields of
> field_info. I propose to make it take field_info pointer instead
> of these 2 arrays. Also for individual fields I propose to add
> field_info_set_field(type, coll_id). So as not to miss anything. Currently
> it looks too much boilerplate code.
> 
Fixed, I think.

> > +			info->types[count] = FIELD_TYPE_UNSIGNED;
> > +			info->coll_ids[count] = COLL_NONE;
> >  		} else {
> > diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> > index b9107fccc..c59d7b4b4 100644
> > --- a/src/box/sql/select.c
> > +++ b/src/box/sql/select.c
> > @@ -99,6 +99,103 @@ 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);
> > +
> > +struct fields_info *
> > +fields_info_new(uint32_t len)
> > +{
> > +	uint32_t size_info = sizeof(struct fields_info);
> > +	uint32_t size_types = len * sizeof(enum field_type);
> > +	uint32_t size = size_info + size_types + len * (sizeof(uint32_t));
> > +	char *buf = sqlDbMallocRawNN(sql_get(), size);
> > +	struct fields_info *info = (struct fields_info *)buf;
> > +	if (info == NULL)
> > +		return NULL;
> > +	info->key_info = NULL;
> > +	buf += size_info;
> > +	info->types = (enum field_type *)buf;
> > +	buf += size_types;
> > +	info->coll_ids = (uint32_t *)buf;
> > +	return info;
> > +}
> > +
> > +void
> > +fields_info_delete(struct fields_info *info)
> > +{
> > +	if (info->key_info != NULL)
> > +		sql_key_info_unref(info->key_info);
> > +	return sqlDbFree(sql_get(), info);
> 
> 6. You do not need this explicit 'return'.
> 
Dropped this function.

> > +}
> > +
> > +void
> > +fields_info_from_space_def(enum field_type *types, uint32_t *coll_ids,
> > +			   uint32_t count, struct space_def *def)
> 
> 7. def can be made const. The same for some other arguments below.
> 
Fixed.

> > +{
> > +	for (uint32_t i = 0; i < count; ++i) {
> > +		types[i] = def->fields[i].type;
> > +		coll_ids[i] = def->fields[i].coll_id;
> > +	}
> > +}
> > +
> > +void
> > +fields_info_from_index_def(enum field_type *types, uint32_t *coll_ids,
> > +			   uint32_t count, struct index_def *def)
> > +{
> > +	for (uint32_t i = 0; i < count; ++i) {
> > +		types[i] = def->key_def->parts[i].type;
> > +		coll_ids[i] = def->key_def->parts[i].coll_id;
> > +	}
> > +}
> > +
> > +static int
> > +fields_info_from_expr_list(enum field_type *types, uint32_t *coll_ids,
> > +			   struct Parse *parser, struct ExprList *list)
> > +{
> > +	for (int i = 0; i < list->nExpr; ++i) {
> > +		bool b;
> > +		struct coll *coll;
> > +		struct Expr *expr = list->a[i].pExpr;
> > +		types[i] = sql_expr_type(expr);
> > +		if (types[i] == FIELD_TYPE_ANY)
> > +			types[i] = FIELD_TYPE_SCALAR;
> > +		if (sql_expr_coll(parser, expr, &b, &coll_ids[i], &coll) != 0)
> > +			return -1;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int
> > +fields_info_from_order_by(enum field_type *types, uint32_t *coll_ids,
> > +			  struct Parse *parser, struct Select *select,
> > +			  struct ExprList *order_by)
> > +{
> > +	for (int i = 0; i < order_by->nExpr; ++i) {
> > +		bool b;
> > +		struct Expr *expr = order_by->a[i].pExpr;
> > +		types[i] = sql_expr_type(expr);
> > +		if (types[i] == FIELD_TYPE_ANY)
> > +			types[i] = FIELD_TYPE_SCALAR;
> > +		uint32_t id = COLL_NONE;
> > +		if ((expr->flags & EP_Collate) != 0) {
> > +			struct coll *coll;
> > +			if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0)
> > +				return -1;
> > +		} else {
> > +			uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
> > +			id = multi_select_coll_seq(parser, select, fieldno);
> > +			if (id != COLL_NONE) {
> > +				const char *name = coll_by_id(id)->name;
> > +				order_by->a[i].pExpr =
> > +					sqlExprAddCollateString(parser, expr,
> > +								name);
> > +			}
> > +		}
> > +		coll_ids[i] = id;
> > +	}
> > +	return 0;
> > +}
> 
> 8. It sligtly cuts the ears that a struct called fields_info also
> defines a key via key_info. It is like specifying indexes inside of
> space format.
> 
> Maybe it is worth renaming it to ephemeral_space_def? Or space_info?
> Or shorten ephemeral to ephmen in all the new code and use
> ephem_space_def? Or sql_space_info? sql_ephemeral_space_def?
> 
> And make sql_ephemeral_space_create() take this struct instead of
> 2 arrays. Besides, you could hide sql_ephemeral_space_create_key_info()
> inside of it because you could check key_info != null in
> sql_ephemeral_space_create(). It just does not seem like vdbe business
> to check these things.
>
Thank you! I renamed the structure to space_info. However, there are times when
I include space_info in key_info (it was the other way around in the last
patch). These are cases where the ephemeral space might be converted to
SorterOpen opcodes or its key_info might change (see pushOntoSorter()).
Actually, I tried to replace key_info everywhere, as there are two cases where
the type of a field can change - when values are inserted into an ephemeral
space and when a sorter is used. However, when I thought I could do this, I
found it all pointless as there is no normal way to get data from the sorter. A
"pseudo" cursor is used to get data from the sorter, and this cursor can only
read tuples. So, after receiving data using a "pseudo" cursor, we lose
information about the SCALAR and NUMBER field types. I think this is not as
problematic as in the case of ephemeral space, where we got the SCALAR after
selecting from ephemeral space. This is a bug, but since the sorter is rarely
used, especially with the SCALAR field, I think we can fix this problem a little
later. In the end, I decided to leave the opcodes that use key_info, other than
OP_OpenTEphemeral, as they are now.
 
> > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> > index ef8dcd693..167e80057 100644
> > --- a/src/box/sql/sqlInt.h
> > +++ b/src/box/sql/sqlInt.h
> > @@ -2248,6 +2248,11 @@ struct Parse {
> >  #define OPFLAG_SYSTEMSP      0x20	/* OP_Open**: set if space pointer
> >  					 * points to system space.
> >  					 */
> > +/**
> > + * If this flag is set, than in OP_OpenTEphemeral we should use rowid as the
> 
> 9. than -> then.
Dropped.

New patch:

commit c445da3d3a55646b9029f2e749e8accb290a13ce
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 b87d236d1..d4b88d6e5 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -243,114 +243,153 @@ 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)
+{
+	assert(info->field_count == def->field_count + 1);
+	assert(info->part_count == 0);
+	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;
+	}
+	info->types[def->field_count] = FIELD_TYPE_INTEGER;
+	info->coll_ids[def->field_count] = COLL_NONE;
+}
+
+void
+sql_space_info_from_index_def(struct space_info *info,
+			      const struct index_def *def)
+{
+	assert(info->field_count >= def->key_def->part_count);
+	assert(info->part_count == 0);
+	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;
+	}
+}
+
+enum {
 	/*
-	 * 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 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);
+	NAME_LEN = 19,
+};
+
+struct space *
+sql_ephemeral_space_new_from_info(const struct 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;
+	uint32_t size = names_indent + field_count * NAME_LEN;
+
+	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 = (void *)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[i * NAME_LEN];
+		sprintf(fields[i].name, "_COLUMN_%d", i);
+		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];
+	}
+	if (info->parts == NULL) {
+		for (uint32_t i = 0; i < part_count; ++i)
+			parts[i].fieldno = i;
+	} else {
+		for (uint32_t i = 0; i < part_count; ++i)
+			parts[i].fieldno = info->parts[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;
+		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].path = NULL;
+		parts[i].type = info->types[parts[i].fieldno];
+		parts[i].coll_id = info->coll_ids[parts[i].fieldno];
 	}
-	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..9d5a63fc5 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -224,12 +224,20 @@ 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 space_info *info = sql_space_info_new(pk_len, 0);
+		if (info == NULL) {
+			parse->is_aborted = true;
+			goto delete_from_cleanup;
+		}
+		parse->nMem += pk_len;
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, pk_len, 0,
+			      (char *)info, P4_DYNAMIC);
 		if (is_view) {
 			/*
 			 * At this stage SELECT is already materialized
@@ -249,21 +257,11 @@ 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);
+			sql_space_info_from_space_def(info, 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;
+			sql_space_info_from_index_def(info, index_def);
 		}
 
 		/* Construct a query to find the primary key for
@@ -295,8 +293,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 +372,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..c67a7091c 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,13 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 			 */
 			pExpr->iTable = pParse->nTab++;
 			int reg_eph = ++pParse->nMem;
-			addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral,
-						 reg_eph, nVal);
+			struct space_info *info = sql_space_info_new(nVal, 0);
+			if (info == NULL)
+				return 0;
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nVal, 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 +2796,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 +2807,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 +2827,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 +2859,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..99b5ceee6 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -442,34 +442,44 @@ 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 space_info *info =
+				sql_space_info_new(nColumn + 1, 1);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				goto insert_cleanup;
 			}
+			struct field_def *fields = space_def->fields;
+			if (pColumn != NULL) {
+				for (int i = 0; i < nColumn; ++i) {
+					int j = pColumn->a[i].idx;
+					info->types[i] =  fields[j].type;
+					info->coll_ids[i] =  fields[j].coll_id;
+				}
+			} else {
+				for (int i = 0; i < nColumn; ++i) {
+					info->types[i] =  fields[i].type;
+					info->coll_ids[i] =  fields[i].coll_id;
+				}
+			}
+			info->types[nColumn] = FIELD_TYPE_INTEGER;
+			info->parts[0] = nColumn;
+
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
+				      nColumn + 1, 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 b9107fccc..a26ca54a3 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -99,6 +99,74 @@ 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 int
+space_info_from_expr_list(struct space_info *info, struct Parse *parser,
+			  struct ExprList *list, int start)
+{
+	assert((int)info->field_count >= list->nExpr - start);
+	for (int i = 0; i < list->nExpr - start; ++i) {
+		bool b;
+		struct coll *coll;
+		struct Expr *expr = list->a[i + 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 -1;
+		info->types[i] = type;
+		info->coll_ids[i] = coll_id;
+	}
+	info->sort_order = list->a[0].sort_order;
+	return 0;
+}
+
+static int
+space_info_from_order_by(struct space_info *info, struct Parse *parser,
+			 struct Select *select, struct ExprList *list)
+{
+	for (int i = 0; i < list->nExpr; ++i) {
+		bool b;
+		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;
+		info->types[i] = type;
+		uint32_t *id = &info->coll_ids[i];
+		if ((expr->flags & EP_Collate) != 0) {
+			struct coll *coll;
+			if (sql_expr_coll(parser, expr, &b, id, &coll) != 0)
+				return -1;
+			continue;
+		}
+		uint32_t fieldno = list->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;
+			list->a[i].pExpr =
+				sqlExprAddCollateString(parser, expr, name);
+		}
+	}
+	info->sort_order = list->a[0].sort_order;
+	info->types[list->nExpr] = FIELD_TYPE_INTEGER;
+	info->types[list->nExpr + 1] = FIELD_TYPE_VARBINARY;
+	return 0;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -823,9 +891,29 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 		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);
+		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,
+							  pSort->pOrderBy,
+							  nOBSat);
+		} else {
+			struct space_info *info =
+				sql_space_info_new(pOp->p2, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return;
+			}
+			pOp->p4.space_info = info;
+			pOp->p4type = P4_DYNAMIC;
+			if (space_info_from_expr_list(info, pParse,
+						      pSort->pOrderBy,
+						      nOBSat) != 0) {
+				pParse->is_aborted = true;
+				return;
+			}
+		}
 		addrJmp = sqlVdbeCurrentAddr(v);
 		sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
 		VdbeCoverage(v);
@@ -1044,13 +1132,28 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 			 * space format.
 			 */
 			uint32_t excess_field_count = 0;
+			struct VdbeOp *op = sqlVdbeGetOp(v,
+							 pSort->addrSortIndex);
+			struct 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;
-				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 =
+					(u16) (i + 1 - pSort->nOBSat);
+				--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 =
@@ -1416,6 +1519,7 @@ 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;
@@ -1444,6 +1548,7 @@ sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def)
 		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;
@@ -1469,6 +1574,7 @@ 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);
 	}
 }
@@ -2453,17 +2559,29 @@ generateWithRecursiveQuery(Parse * pParse,	/* Parsing context */
 	/* Allocate cursors for Current, Queue, and Distinct. */
 	regCurrent = ++pParse->nMem;
 	sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol);
+	int field_count = pOrderBy != NULL ? pOrderBy->nExpr + 2 : nCol + 1;
+	struct space_info *info = sql_space_info_new(field_count, 0);
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto end_of_recursive_query;
+	}
+	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, field_count, 0,
+		      (char *)info, P4_DYNAMIC);
 	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"));
+		if (space_info_from_order_by(info, pParse, p, pOrderBy) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
 		destQueue.pOrderBy = pOrderBy;
 	} else {
-		sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1);
 		VdbeComment((v, "Queue table"));
+		if (space_info_from_expr_list(info, pParse, p->pEList,
+					      0) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
+		info->types[nCol] = FIELD_TYPE_INTEGER;
 	}
 	sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
 	if (iDistinct) {
@@ -2673,7 +2791,21 @@ 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 space_info *info = sql_space_info_new(nCols + 1, 0);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1,
+			      0, (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, p->pEList,
+					      0) != 0) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		info->types[nCols] = FIELD_TYPE_INTEGER;
 		sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
 		VdbeComment((v, "Destination temp"));
 		dest.eDest = SRT_Table;
@@ -3001,14 +3133,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 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 +3152,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 +5481,31 @@ 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,
-								  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);
+				return;
+			}
+			assert(pE->x.pList->nExpr == 1);
+			struct space_info *info = sql_space_info_new(1, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
 			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1,
+				      0, (char *)info, P4_DYNAMIC);
+			bool b;
+			struct coll *coll;
+			struct Expr *expr = pE->x.pList->a[0].pExpr;
+			info->types[0] = sql_expr_type(expr);
+			if (info->types[0] == FIELD_TYPE_ANY)
+				info->types[0] = FIELD_TYPE_SCALAR;
+			if (sql_expr_coll(pParse, expr, &b, &info->coll_ids[0],
+					  &coll) != 0) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
+			}
+			sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0,
+				      pFunc->reg_eph);
 		}
 	}
 }
@@ -5879,6 +6027,39 @@ 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;
+		struct ExprList *list = sSort.pOrderBy;
+		if (space_info_from_expr_list(info, pParse, list, 0) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		info->types[sSort.pOrderBy->nExpr] = FIELD_TYPE_INTEGER;
+		for (int i = 0; i < pEList->nExpr; ++i) {
+			bool b;
+			struct Expr *expr = pEList->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(pParse, expr, &b, &id, &coll) != 0) {
+				pParse->is_aborted = true;
+				goto select_end;
+			}
+			int j = part_count + i;
+			info->types[j] = type;
+			info->coll_ids[j] = id;
+		}
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
 	} else {
@@ -5888,11 +6069,19 @@ 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);
+		uint32_t cols = pEList->nExpr + 1;
+		struct space_info *info = sql_space_info_new(cols, 0);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, cols, 0,
+			      (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		info->types[pEList->nExpr] = FIELD_TYPE_INTEGER;
 		sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
 				  pDest->reg_eph);
 
@@ -5918,13 +6107,19 @@ 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);
+		uint32_t cols = pEList->nExpr;
+		struct space_info *info = sql_space_info_new(cols, 0);
+		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, cols, 0,
+						   (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		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..60fa1678d 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4006,6 +4006,8 @@ sql_analysis_load(struct sql *db);
  */
 struct sql_key_info {
 	sql *db;
+	/* Informations about all field types and key parts. */
+	struct space_info *info;
 	/**
 	 * Key definition created from this object,
 	 * see sql_key_info_to_key_def().
@@ -4055,6 +4057,66 @@ 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 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;
+	/** 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;
+	/** Sort order of index. */
+	enum sort_order sort_order;
+};
+
+/**
+ * Allocate and initialize with default values a structure that will be used to
+ * store information about ephemeral space field types and key parts.
+ */
+struct 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.
+ */
+void
+sql_space_info_from_space_def(struct space_info *info,
+			      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.
+ */
+void
+sql_space_info_from_index_def(struct space_info *info,
+			      const struct index_def *def);
+
+/**
+ * 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_from_info(const struct 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..8eab8a3df 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -226,9 +226,21 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	iEph = pParse->nTab++;
 	sqlVdbeAddOp2(v, OP_Null, 0, iPk);
 
+	struct space_info *info = sql_space_info_new(pk_part_count, 0);
+	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,
+				     pk_part_count, 0, (char *)info,
+				     P4_DYNAMIC);
+	assert(space->index_count > 0 || is_view);
+	if (is_view)
+		sql_space_info_from_space_def(info, def);
+	else
+		sql_space_info_from_index_def(info, pPk->def);
+
 	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..14caa6f82 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;
 }
@@ -2392,10 +2390,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);
+	struct space_info *info = pOp->p4type == P4_KEYINFO ?
+				  pOp->p4.key_info->info : pOp->p4.space_info;
+	assert(info != NULL);
+	struct space *space = sql_ephemeral_space_new_from_info(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..505892861 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 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..31490a9b2 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,15 +919,16 @@ 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 space_info *info = sql_space_info_new(nKeyCol + 1, 0);
+	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);
+		      (char *)info, P4_DYNAMIC);
+	sql_space_info_from_index_def(info, idx_def);
+	info->types[nKeyCol] = FIELD_TYPE_INTEGER;
 	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..a8a1893d3 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 space_info *info =
+				sql_space_info_new(pk_part_count, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return notReady;
+			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set,
+				      pk_part_count, 0, (char *)info,
+				      P4_DYNAMIC);
+			struct index_def *index_def = space->index[0]->def;
+			sql_space_info_from_index_def(info, index_def);
 			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);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
  2021-08-10 14:32   ` Mergen Imeev via Tarantool-patches
@ 2021-08-12 18:32     ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-12 18:56       ` Safin Timur via Tarantool-patches
  2021-08-12 21:49       ` Mergen Imeev via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-08-12 18:32 UTC (permalink / raw)
  To: Mergen Imeev; +Cc: tarantool-patches

Hi! Thanks for the fixes!

See 7 comments below.

> diff --git a/src/box/sql.c b/src/box/sql.c
> index b87d236d1..d4b88d6e5 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c

<...>

> +void
> +sql_space_info_from_space_def(struct space_info *info,
> +			      const struct space_def *def)
> +{
> +	assert(info->field_count == def->field_count + 1);
> +	assert(info->part_count == 0);
> +	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;
> +	}
> +	info->types[def->field_count] = FIELD_TYPE_INTEGER;
> +	info->coll_ids[def->field_count] = COLL_NONE;

1. Please, explain in a comment what is this last special field.

<...>

> +struct space *
> +sql_ephemeral_space_new_from_info(const struct 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;
> +	uint32_t size = names_indent + field_count * NAME_LEN;
> +
> +	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 = (void *)fields + parts_indent;

2. Are you sure void * + integer works bytewise? Shouldn't it be
cast to char * to move the pointer? In other places you use char *.

> +	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[i * NAME_LEN];
> +		sprintf(fields[i].name, "_COLUMN_%d", i);
> +		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];
> +	}
> +	if (info->parts == NULL) {
> +		for (uint32_t i = 0; i < part_count; ++i)
> +			parts[i].fieldno = i;
> +	} else {
> +		for (uint32_t i = 0; i < part_count; ++i)
> +			parts[i].fieldno = info->parts[i];
>  	}

3. It looks not so good for the cache that you walk the parts
array more than once.

> -
>  	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;
> +		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].path = NULL;
> +		parts[i].type = info->types[parts[i].fieldno];
> +		parts[i].coll_id = info->coll_ids[parts[i].fieldno];
>  	}
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index 5226dd6ea..9d5a63fc5 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -224,12 +224,20 @@ 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 space_info *info = sql_space_info_new(pk_len, 0);

4. It is hard to understand when you calculate the space info field
count, create the info, and later sql_space_info_from_space_def() assumes
internally the field count you calculated manually. But I can't propose
anything better now.

> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 21b4f2407..99b5ceee6 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -442,34 +442,44 @@ 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 space_info *info =
> +				sql_space_info_new(nColumn + 1, 1);
> +			if (info == NULL) {
> +				pParse->is_aborted = true;
> +				goto insert_cleanup;
>  			}
> +			struct field_def *fields = space_def->fields;
> +			if (pColumn != NULL) {
> +				for (int i = 0; i < nColumn; ++i) {
> +					int j = pColumn->a[i].idx;
> +					info->types[i] =  fields[j].type;
> +					info->coll_ids[i] =  fields[j].coll_id;
> +				}
> +			} else {
> +				for (int i = 0; i < nColumn; ++i) {
> +					info->types[i] =  fields[i].type;
> +					info->coll_ids[i] =  fields[i].coll_id;
> +				}
> +			}
> +			info->types[nColumn] = FIELD_TYPE_INTEGER;
> +			info->parts[0] = nColumn;

5. Why is sql_space_info_from_space_def() extracted but this is not?
It looks quite internal to space_info, since it changes its fields in
some intricate way.

The same in the other places where info is changed. For
instance, in @@ -5879,6 +6027,39 @@ sqlSelect.

> +
> +			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
> +				      nColumn + 1, 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 b9107fccc..a26ca54a3 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -1044,13 +1132,28 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
>  			 * space format.
>  			 */
>  			uint32_t excess_field_count = 0;
> +			struct VdbeOp *op = sqlVdbeGetOp(v,
> +							 pSort->addrSortIndex);
> +			struct 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;
> -				if (j > 0) {
> -					excess_field_count++;
> -					pEList->a[j - 1].u.x.iOrderByCol =
> -						(u16) (i + 1 - pSort->nOBSat);

6. No need for a whitespace after the cast. Also it could be
turned to uint16_t since you changed the line anyway.

> +				if (j == 0)
> +					continue;
> +				assert(j > 0);
> +				excess_field_count++;
> +				pEList->a[j - 1].u.x.iOrderByCol =
> +					(u16) (i + 1 - pSort->nOBSat);
> +				--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];
>  				}
>  			}
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 1f87e6823..60fa1678d 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -4006,6 +4006,8 @@ sql_analysis_load(struct sql *db);
>   */
>  struct sql_key_info {
>  	sql *db;
> +	/* Informations about all field types and key parts. */

7. For out of comments we use /** opening usually.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
  2021-08-12 18:32     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-08-12 18:56       ` Safin Timur via Tarantool-patches
  2021-08-12 21:49       ` Mergen Imeev via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Safin Timur via Tarantool-patches @ 2021-08-12 18:56 UTC (permalink / raw)
  To: Vladislav Shpilevoy, Mergen Imeev; +Cc: tarantool-patches


On 12.08.2021 21:32, Vladislav Shpilevoy via Tarantool-patches wrote:
> Hi! Thanks for the fixes!
> 
> See 7 comments below.
> 
>> -		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 = (void *)fields + parts_indent;
> 
> 2. Are you sure void * + integer works bytewise? Shouldn't it be
> cast to char * to move the pointer? In other places you use char *.

Good point. Such void pointer arithmetic is gcc specific language 
extension (https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html), and 
is not a standard approach.

Timur

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces
  2021-08-12 18:32     ` Vladislav Shpilevoy via Tarantool-patches
  2021-08-12 18:56       ` Safin Timur via Tarantool-patches
@ 2021-08-12 21:49       ` Mergen Imeev via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-12 21:49 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hi! Thank you for the review! Also, I am really sorry that my last commist are
quite far expected quality. In part this is because I plan to rework this code
- I want to divide "prepare" code to parser, resolver and VDBE constructor.
Another reason that I spent too much time on my tasks and now have to implement
them as fast as I can (well, this is as usual before release).

I fixed you you comments, however there are some comments to my fixed. See my
answers, diff and new patch below.

On Thu, Aug 12, 2021 at 09:32:38PM +0300, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
> 
> See 7 comments below.
> 
> > diff --git a/src/box/sql.c b/src/box/sql.c
> > index b87d236d1..d4b88d6e5 100644
> > --- a/src/box/sql.c
> > +++ b/src/box/sql.c
> 
> <...>
> 
> > +void
> > +sql_space_info_from_space_def(struct space_info *info,
> > +			      const struct space_def *def)
> > +{
> > +	assert(info->field_count == def->field_count + 1);
> > +	assert(info->part_count == 0);
> > +	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;
> > +	}
> > +	info->types[def->field_count] = FIELD_TYPE_INTEGER;
> > +	info->coll_ids[def->field_count] = COLL_NONE;
> 
> 1. Please, explain in a comment what is this last special field.
> 
Fixed.

> <...>
> 
> > +struct space *
> > +sql_ephemeral_space_new_from_info(const struct 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;
> > +	uint32_t size = names_indent + field_count * NAME_LEN;
> > +
> > +	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 = (void *)fields + parts_indent;
> 
> 2. Are you sure void * + integer works bytewise? Shouldn't it be
> cast to char * to move the pointer? In other places you use char *.
>
Fixed. I copied this part from sql_ephemeral_space_create() and thought about
using this approach everywhere in this function. However, at the end decided
against it and used usual approach. Forgot to change here.

> > +	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[i * NAME_LEN];
> > +		sprintf(fields[i].name, "_COLUMN_%d", i);
> > +		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];
> > +	}
> > +	if (info->parts == NULL) {
> > +		for (uint32_t i = 0; i < part_count; ++i)
> > +			parts[i].fieldno = i;
> > +	} else {
> > +		for (uint32_t i = 0; i < part_count; ++i)
> > +			parts[i].fieldno = info->parts[i];
> >  	}
> 
> 3. It looks not so good for the cache that you walk the parts
> array more than once.
> 
Fixed.

> > -
> >  	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;
> > +		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].path = NULL;
> > +		parts[i].type = info->types[parts[i].fieldno];
> > +		parts[i].coll_id = info->coll_ids[parts[i].fieldno];
> >  	}
> > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> > index 5226dd6ea..9d5a63fc5 100644
> > --- a/src/box/sql/delete.c
> > +++ b/src/box/sql/delete.c
> > @@ -224,12 +224,20 @@ 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 space_info *info = sql_space_info_new(pk_len, 0);
> 
> 4. It is hard to understand when you calculate the space info field
> count, create the info, and later sql_space_info_from_space_def() assumes
> internally the field count you calculated manually. But I can't propose
> anything better now.
> 
> > diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> > index 21b4f2407..99b5ceee6 100644
> > --- a/src/box/sql/insert.c
> > +++ b/src/box/sql/insert.c
> > @@ -442,34 +442,44 @@ 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 space_info *info =
> > +				sql_space_info_new(nColumn + 1, 1);
> > +			if (info == NULL) {
> > +				pParse->is_aborted = true;
> > +				goto insert_cleanup;
> >  			}
> > +			struct field_def *fields = space_def->fields;
> > +			if (pColumn != NULL) {
> > +				for (int i = 0; i < nColumn; ++i) {
> > +					int j = pColumn->a[i].idx;
> > +					info->types[i] =  fields[j].type;
> > +					info->coll_ids[i] =  fields[j].coll_id;
> > +				}
> > +			} else {
> > +				for (int i = 0; i < nColumn; ++i) {
> > +					info->types[i] =  fields[i].type;
> > +					info->coll_ids[i] =  fields[i].coll_id;
> > +				}
> > +			}
> > +			info->types[nColumn] = FIELD_TYPE_INTEGER;
> > +			info->parts[0] = nColumn;
> 
> 5. Why is sql_space_info_from_space_def() extracted but this is not?
> It looks quite internal to space_info, since it changes its fields in
> some intricate way.
> 
Added a new static function.

> The same in the other places where info is changed. For
> instance, in @@ -5879,6 +6027,39 @@ sqlSelect.
> 
Added a new static function, however there is still some places where types or
collations are changed. I am not sure that we have to change them, since now it
is quite easy to understand what is going on (see select.c, lines 1178-1184 and
3166-3167).

> > +
> > +			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
> > +				      nColumn + 1, 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 b9107fccc..a26ca54a3 100644
> > --- a/src/box/sql/select.c
> > +++ b/src/box/sql/select.c
> > @@ -1044,13 +1132,28 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
> >  			 * space format.
> >  			 */
> >  			uint32_t excess_field_count = 0;
> > +			struct VdbeOp *op = sqlVdbeGetOp(v,
> > +							 pSort->addrSortIndex);
> > +			struct 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;
> > -				if (j > 0) {
> > -					excess_field_count++;
> > -					pEList->a[j - 1].u.x.iOrderByCol =
> > -						(u16) (i + 1 - pSort->nOBSat);
> 
> 6. No need for a whitespace after the cast. Also it could be
> turned to uint16_t since you changed the line anyway.
> 
Fixed.

> > +				if (j == 0)
> > +					continue;
> > +				assert(j > 0);
> > +				excess_field_count++;
> > +				pEList->a[j - 1].u.x.iOrderByCol =
> > +					(u16) (i + 1 - pSort->nOBSat);
> > +				--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];
> >  				}
> >  			}
> > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> > index 1f87e6823..60fa1678d 100644
> > --- a/src/box/sql/sqlInt.h
> > +++ b/src/box/sql/sqlInt.h
> > @@ -4006,6 +4006,8 @@ sql_analysis_load(struct sql *db);
> >   */
> >  struct sql_key_info {
> >  	sql *db;
> > +	/* Informations about all field types and key parts. */
> 
> 7. For out of comments we use /** opening usually.
> 
Fixed.


Diff:

diff --git a/src/box/sql.c b/src/box/sql.c
index 9042c5bb2..2b95aed8d 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -285,6 +285,7 @@ sql_space_info_from_space_def(struct space_info *info,
 		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;
 	info->coll_ids[def->field_count] = COLL_NONE;
 }
@@ -329,7 +330,8 @@ sql_ephemeral_space_new_from_info(const struct space_info *info)
 		diag_set(OutOfMemory, size, "region_aligned_alloc", "fields");
 		return NULL;
 	}
-	struct key_part_def *parts = (void *)fields + parts_indent;
+	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;
@@ -345,20 +347,28 @@ sql_ephemeral_space_new_from_info(const struct space_info *info)
 		fields[i].coll_id = info->coll_ids[i];
 	}
 	if (info->parts == NULL) {
-		for (uint32_t i = 0; i < part_count; ++i)
+		for (uint32_t i = 0; i < part_count; ++i) {
 			parts[i].fieldno = i;
+			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].path = NULL;
+			parts[i].type = info->types[i];
+			parts[i].coll_id = info->coll_ids[i];
+		}
 	} else {
-		for (uint32_t i = 0; i < part_count; ++i)
-			parts[i].fieldno = info->parts[i];
-	}
-	for (uint32_t i = 0; i < part_count; ++i) {
-		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].path = NULL;
-		parts[i].type = info->types[parts[i].fieldno];
-		parts[i].coll_id = info->coll_ids[parts[i].fieldno];
+		for (uint32_t i = 0; i < part_count; ++i) {
+			uint32_t j = 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 = info->sort_order;
+			parts[i].path = NULL;
+			parts[i].type = info->types[j];
+			parts[i].coll_id = info->coll_ids[j];
+		}
 	}
 
 	struct key_def *key_def = key_def_new(parts, part_count, false);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 99b5ceee6..80579c1d2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -60,6 +60,31 @@ sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg)
 			  (char *)colls_type, P4_DYNAMIC);
 }
 
+static void
+sql_space_info_from_id_list(struct space_info *info,
+			    const struct space_def *def,
+			    const struct IdList *list)
+{
+	int n = info->field_count - 1;
+	struct field_def *fields = def->fields;
+	if (list != NULL) {
+		for (int i = 0; i < n; ++i) {
+			int j = list->a[i].idx;
+			info->types[i] =  fields[j].type;
+			info->coll_ids[i] =  fields[j].coll_id;
+		}
+	} else {
+		for (int i = 0; i < n; ++i) {
+			info->types[i] =  fields[i].type;
+			info->coll_ids[i] =  fields[i].coll_id;
+		}
+	}
+	/* Add one more field for rowid. */
+	info->types[n] = FIELD_TYPE_INTEGER;
+	/* Set rowid as the only part of primary index. */
+	info->parts[0] = n;
+}
+
 /**
  * In SQL table can be created with AUTOINCREMENT.
  * In Tarantool it can be detected as primary key which consists
@@ -455,21 +480,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 				pParse->is_aborted = true;
 				goto insert_cleanup;
 			}
-			struct field_def *fields = space_def->fields;
-			if (pColumn != NULL) {
-				for (int i = 0; i < nColumn; ++i) {
-					int j = pColumn->a[i].idx;
-					info->types[i] =  fields[j].type;
-					info->coll_ids[i] =  fields[j].coll_id;
-				}
-			} else {
-				for (int i = 0; i < nColumn; ++i) {
-					info->types[i] =  fields[i].type;
-					info->coll_ids[i] =  fields[i].coll_id;
-				}
-			}
-			info->types[nColumn] = FIELD_TYPE_INTEGER;
-			info->parts[0] = nColumn;
+			sql_space_info_from_id_list(info, space_def, pColumn);
 
 			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
 				      nColumn + 1, 0, (char *)info, P4_DYNAMIC);
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index a26ca54a3..a3e551017 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -104,10 +104,11 @@ multi_select_coll_seq(struct Parse *parser, struct Select *p, int n);
 
 static int
 space_info_from_expr_list(struct space_info *info, struct Parse *parser,
-			  struct ExprList *list, int start)
+			  struct ExprList *list, int start, bool has_rowid)
 {
 	assert((int)info->field_count >= list->nExpr - start);
-	for (int i = 0; i < list->nExpr - start; ++i) {
+	int n = list->nExpr - start;
+	for (int i = 0; i < n; ++i) {
 		bool b;
 		struct coll *coll;
 		struct Expr *expr = list->a[i + start].pExpr;
@@ -126,6 +127,8 @@ space_info_from_expr_list(struct space_info *info, struct Parse *parser,
 		info->coll_ids[i] = coll_id;
 	}
 	info->sort_order = list->a[0].sort_order;
+	if (has_rowid)
+		info->types[n] = FIELD_TYPE_INTEGER;
 	return 0;
 }
 
@@ -167,6 +170,30 @@ space_info_from_order_by(struct space_info *info, struct Parse *parser,
 	return 0;
 }
 
+static int
+space_info_for_sorting(struct space_info *info, struct Parse *parser,
+		       struct ExprList *order_by, struct ExprList *list)
+{
+	if (space_info_from_expr_list(info, parser, order_by, 0, true) != 0)
+		return -1;
+	uint32_t part_count = order_by->nExpr + 1;
+	for (int i = 0; i < list->nExpr; ++i) {
+		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 -1;
+		int j = part_count + i;
+		info->types[j] = type;
+		info->coll_ids[j] = id;
+	}
+	return 0;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -909,7 +936,7 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 			pOp->p4type = P4_DYNAMIC;
 			if (space_info_from_expr_list(info, pParse,
 						      pSort->pOrderBy,
-						      nOBSat) != 0) {
+						      nOBSat, false) != 0) {
 				pParse->is_aborted = true;
 				return;
 			}
@@ -1147,7 +1174,7 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 				assert(j > 0);
 				excess_field_count++;
 				pEList->a[j - 1].u.x.iOrderByCol =
-					(u16) (i + 1 - pSort->nOBSat);
+					(uint16_t)(i + 1 - pSort->nOBSat);
 				--info->field_count;
 				for (int k = j; k < pEList->nExpr; ++k) {
 					int n = k + pSort->pOrderBy->nExpr + 1;
@@ -2577,11 +2604,10 @@ generateWithRecursiveQuery(Parse * pParse,	/* Parsing context */
 	} else {
 		VdbeComment((v, "Queue table"));
 		if (space_info_from_expr_list(info, pParse, p->pEList,
-					      0) != 0) {
+					      0, true) != 0) {
 			pParse->is_aborted = true;
 			goto end_of_recursive_query;
 		}
-		info->types[nCol] = FIELD_TYPE_INTEGER;
 	}
 	sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
 	if (iDistinct) {
@@ -2800,12 +2826,11 @@ multiSelect(Parse * pParse,	/* Parsing context */
 		sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1,
 			      0, (char *)info, P4_DYNAMIC);
 		if (space_info_from_expr_list(info, pParse, p->pEList,
-					      0) != 0) {
+					      0, true) != 0) {
 			pParse->is_aborted = true;
 			rc = 1;
 			goto multi_select_end;
 		}
-		info->types[nCols] = FIELD_TYPE_INTEGER;
 		sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
 		VdbeComment((v, "Destination temp"));
 		dest.eDest = SRT_Table;
@@ -5492,14 +5517,9 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo)
 			}
 			sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1,
 				      0, (char *)info, P4_DYNAMIC);
-			bool b;
-			struct coll *coll;
-			struct Expr *expr = pE->x.pList->a[0].pExpr;
-			info->types[0] = sql_expr_type(expr);
-			if (info->types[0] == FIELD_TYPE_ANY)
-				info->types[0] = FIELD_TYPE_SCALAR;
-			if (sql_expr_coll(pParse, expr, &b, &info->coll_ids[0],
-					  &coll) != 0) {
+			if (space_info_from_expr_list(info, pParse,
+						      pE->x.pList, 0,
+						      false) != 0) {
 				pParse->is_aborted = true;
 				pFunc->iDistinct = -1;
 				return;
@@ -6038,28 +6058,12 @@ sqlSelect(Parse * pParse,		/* The parser context */
 			goto select_end;
 		}
 		key_info->info = info;
-		struct ExprList *list = sSort.pOrderBy;
-		if (space_info_from_expr_list(info, pParse, list, 0) != 0) {
+		if (space_info_for_sorting(info, pParse, sSort.pOrderBy,
+					   pEList) != 0) {
 			pParse->is_aborted = true;
 			goto select_end;
 		}
-		info->types[sSort.pOrderBy->nExpr] = FIELD_TYPE_INTEGER;
-		for (int i = 0; i < pEList->nExpr; ++i) {
-			bool b;
-			struct Expr *expr = pEList->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(pParse, expr, &b, &id, &coll) != 0) {
-				pParse->is_aborted = true;
-				goto select_end;
-			}
-			int j = part_count + i;
-			info->types[j] = type;
-			info->coll_ids[j] = id;
-		}
+
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
 	} else {
@@ -6077,11 +6081,11 @@ sqlSelect(Parse * pParse,		/* The parser context */
 		}
 		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, cols, 0,
 			      (char *)info, P4_DYNAMIC);
-		if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) {
+		if (space_info_from_expr_list(info, pParse, pEList, 0,
+					      true) != 0) {
 			pParse->is_aborted = true;
 			goto select_end;
 		}
-		info->types[pEList->nExpr] = FIELD_TYPE_INTEGER;
 		sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
 				  pDest->reg_eph);
 
@@ -6116,7 +6120,8 @@ sqlSelect(Parse * pParse,		/* The parser context */
 		sDistinct.addrTnct = sqlVdbeAddOp4(v, OP_OpenTEphemeral,
 						   sDistinct.reg_eph, cols, 0,
 						   (char *)info, P4_DYNAMIC);
-		if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) {
+		if (space_info_from_expr_list(info, pParse, pEList, 0,
+					      false) != 0) {
 			pParse->is_aborted = true;
 			goto select_end;
 		}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 60fa1678d..d854562d0 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4006,7 +4006,7 @@ sql_analysis_load(struct sql *db);
  */
 struct sql_key_info {
 	sql *db;
-	/* Informations about all field types and key parts. */
+	/** Informations about all field types and key parts. */
 	struct space_info *info;
 	/**
 	 * Key definition created from this object,


New patch:


commit 20eb7c0115c8b3c90386da95a0ddf64dccbd6b0f
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 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)
+{
+	assert(info->field_count == def->field_count + 1);
+	assert(info->part_count == 0);
+	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;
+	info->coll_ids[def->field_count] = COLL_NONE;
+}
+
+void
+sql_space_info_from_index_def(struct space_info *info,
+			      const struct index_def *def)
+{
+	assert(info->field_count >= def->key_def->part_count);
+	assert(info->part_count == 0);
+	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;
+	}
+}
+
+enum {
 	/*
-	 * 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 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);
+	NAME_LEN = 19,
+};
+
+struct space *
+sql_ephemeral_space_new_from_info(const struct 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;
+	uint32_t size = names_indent + field_count * NAME_LEN;
+
+	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[i * NAME_LEN];
+		sprintf(fields[i].name, "_COLUMN_%d", i);
+		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];
+	}
+	if (info->parts == NULL) {
+		for (uint32_t i = 0; i < part_count; ++i) {
+			parts[i].fieldno = i;
+			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].path = NULL;
+			parts[i].type = info->types[i];
+			parts[i].coll_id = info->coll_ids[i];
+		}
+	} else {
+		for (uint32_t i = 0; i < part_count; ++i) {
+			uint32_t j = 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 = info->sort_order;
+			parts[i].path = NULL;
+			parts[i].type = info->types[j];
+			parts[i].coll_id = info->coll_ids[j];
 		}
 	}
 
-	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;
-	}
-	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..9d5a63fc5 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -224,12 +224,20 @@ 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 space_info *info = sql_space_info_new(pk_len, 0);
+		if (info == NULL) {
+			parse->is_aborted = true;
+			goto delete_from_cleanup;
+		}
+		parse->nMem += pk_len;
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, pk_len, 0,
+			      (char *)info, P4_DYNAMIC);
 		if (is_view) {
 			/*
 			 * At this stage SELECT is already materialized
@@ -249,21 +257,11 @@ 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);
+			sql_space_info_from_space_def(info, 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;
+			sql_space_info_from_index_def(info, index_def);
 		}
 
 		/* Construct a query to find the primary key for
@@ -295,8 +293,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 +372,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..c67a7091c 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,13 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 			 */
 			pExpr->iTable = pParse->nTab++;
 			int reg_eph = ++pParse->nMem;
-			addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral,
-						 reg_eph, nVal);
+			struct space_info *info = sql_space_info_new(nVal, 0);
+			if (info == NULL)
+				return 0;
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nVal, 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 +2796,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 +2807,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 +2827,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 +2859,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..80579c1d2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -60,6 +60,31 @@ sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg)
 			  (char *)colls_type, P4_DYNAMIC);
 }
 
+static void
+sql_space_info_from_id_list(struct space_info *info,
+			    const struct space_def *def,
+			    const struct IdList *list)
+{
+	int n = info->field_count - 1;
+	struct field_def *fields = def->fields;
+	if (list != NULL) {
+		for (int i = 0; i < n; ++i) {
+			int j = list->a[i].idx;
+			info->types[i] =  fields[j].type;
+			info->coll_ids[i] =  fields[j].coll_id;
+		}
+	} else {
+		for (int i = 0; i < n; ++i) {
+			info->types[i] =  fields[i].type;
+			info->coll_ids[i] =  fields[i].coll_id;
+		}
+	}
+	/* Add one more field for rowid. */
+	info->types[n] = FIELD_TYPE_INTEGER;
+	/* Set rowid as the only part of primary index. */
+	info->parts[0] = n;
+}
+
 /**
  * In SQL table can be created with AUTOINCREMENT.
  * In Tarantool it can be detected as primary key which consists
@@ -442,34 +467,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 space_info *info =
+				sql_space_info_new(nColumn + 1, 1);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				goto insert_cleanup;
 			}
+			sql_space_info_from_id_list(info, space_def, pColumn);
+
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
+				      nColumn + 1, 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 b9107fccc..a3e551017 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -99,6 +99,101 @@ 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 int
+space_info_from_expr_list(struct space_info *info, struct Parse *parser,
+			  struct ExprList *list, int start, bool has_rowid)
+{
+	assert((int)info->field_count >= list->nExpr - start);
+	int n = list->nExpr - start;
+	for (int i = 0; i < n; ++i) {
+		bool b;
+		struct coll *coll;
+		struct Expr *expr = list->a[i + 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 -1;
+		info->types[i] = type;
+		info->coll_ids[i] = coll_id;
+	}
+	info->sort_order = list->a[0].sort_order;
+	if (has_rowid)
+		info->types[n] = FIELD_TYPE_INTEGER;
+	return 0;
+}
+
+static int
+space_info_from_order_by(struct space_info *info, struct Parse *parser,
+			 struct Select *select, struct ExprList *list)
+{
+	for (int i = 0; i < list->nExpr; ++i) {
+		bool b;
+		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;
+		info->types[i] = type;
+		uint32_t *id = &info->coll_ids[i];
+		if ((expr->flags & EP_Collate) != 0) {
+			struct coll *coll;
+			if (sql_expr_coll(parser, expr, &b, id, &coll) != 0)
+				return -1;
+			continue;
+		}
+		uint32_t fieldno = list->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;
+			list->a[i].pExpr =
+				sqlExprAddCollateString(parser, expr, name);
+		}
+	}
+	info->sort_order = list->a[0].sort_order;
+	info->types[list->nExpr] = FIELD_TYPE_INTEGER;
+	info->types[list->nExpr + 1] = FIELD_TYPE_VARBINARY;
+	return 0;
+}
+
+static int
+space_info_for_sorting(struct space_info *info, struct Parse *parser,
+		       struct ExprList *order_by, struct ExprList *list)
+{
+	if (space_info_from_expr_list(info, parser, order_by, 0, true) != 0)
+		return -1;
+	uint32_t part_count = order_by->nExpr + 1;
+	for (int i = 0; i < list->nExpr; ++i) {
+		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 -1;
+		int j = part_count + i;
+		info->types[j] = type;
+		info->coll_ids[j] = id;
+	}
+	return 0;
+}
+
 /*
  * Delete all the content of a Select structure.  Deallocate the structure
  * itself only if bFree is true.
@@ -823,9 +918,29 @@ pushOntoSorter(Parse * pParse,		/* Parser context */
 		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);
+		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,
+							  pSort->pOrderBy,
+							  nOBSat);
+		} else {
+			struct space_info *info =
+				sql_space_info_new(pOp->p2, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return;
+			}
+			pOp->p4.space_info = info;
+			pOp->p4type = P4_DYNAMIC;
+			if (space_info_from_expr_list(info, pParse,
+						      pSort->pOrderBy,
+						      nOBSat, false) != 0) {
+				pParse->is_aborted = true;
+				return;
+			}
+		}
 		addrJmp = sqlVdbeCurrentAddr(v);
 		sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
 		VdbeCoverage(v);
@@ -1044,13 +1159,28 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 			 * space format.
 			 */
 			uint32_t excess_field_count = 0;
+			struct VdbeOp *op = sqlVdbeGetOp(v,
+							 pSort->addrSortIndex);
+			struct 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;
-				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);
+				--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 =
@@ -1416,6 +1546,7 @@ 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;
@@ -1444,6 +1575,7 @@ sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def)
 		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;
@@ -1469,6 +1601,7 @@ 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);
 	}
 }
@@ -2453,17 +2586,28 @@ generateWithRecursiveQuery(Parse * pParse,	/* Parsing context */
 	/* Allocate cursors for Current, Queue, and Distinct. */
 	regCurrent = ++pParse->nMem;
 	sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol);
+	int field_count = pOrderBy != NULL ? pOrderBy->nExpr + 2 : nCol + 1;
+	struct space_info *info = sql_space_info_new(field_count, 0);
+	if (info == NULL) {
+		pParse->is_aborted = true;
+		goto end_of_recursive_query;
+	}
+	sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, field_count, 0,
+		      (char *)info, P4_DYNAMIC);
 	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"));
+		if (space_info_from_order_by(info, pParse, p, pOrderBy) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
 		destQueue.pOrderBy = pOrderBy;
 	} else {
-		sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1);
 		VdbeComment((v, "Queue table"));
+		if (space_info_from_expr_list(info, pParse, p->pEList,
+					      0, true) != 0) {
+			pParse->is_aborted = true;
+			goto end_of_recursive_query;
+		}
 	}
 	sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
 	if (iDistinct) {
@@ -2673,7 +2817,20 @@ 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 space_info *info = sql_space_info_new(nCols + 1, 0);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1,
+			      0, (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, p->pEList,
+					      0, true) != 0) {
+			pParse->is_aborted = true;
+			rc = 1;
+			goto multi_select_end;
+		}
 		sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
 		VdbeComment((v, "Destination temp"));
 		dest.eDest = SRT_Table;
@@ -3001,14 +3158,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 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 +3177,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 +5506,26 @@ 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,
-								  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);
+				return;
+			}
+			assert(pE->x.pList->nExpr == 1);
+			struct space_info *info = sql_space_info_new(1, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
 			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1,
+				      0, (char *)info, P4_DYNAMIC);
+			if (space_info_from_expr_list(info, pParse,
+						      pE->x.pList, 0,
+						      false) != 0) {
+				pParse->is_aborted = true;
+				pFunc->iDistinct = -1;
+				return;
+			}
+			sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0,
+				      pFunc->reg_eph);
 		}
 	}
 }
@@ -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;
+		}
+
 		sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
 		VdbeComment((v, "Sort table"));
 	} else {
@@ -5888,11 +6073,19 @@ 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);
+		uint32_t cols = pEList->nExpr + 1;
+		struct space_info *info = sql_space_info_new(cols, 0);
+		if (info == NULL) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
+		sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, cols, 0,
+			      (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, pEList, 0,
+					      true) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
 				  pDest->reg_eph);
 
@@ -5918,13 +6111,20 @@ 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);
+		uint32_t cols = pEList->nExpr;
+		struct space_info *info = sql_space_info_new(cols, 0);
+		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, cols, 0,
+						   (char *)info, P4_DYNAMIC);
+		if (space_info_from_expr_list(info, pParse, pEList, 0,
+					      false) != 0) {
+			pParse->is_aborted = true;
+			goto select_end;
+		}
 		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..d854562d0 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4006,6 +4006,8 @@ sql_analysis_load(struct sql *db);
  */
 struct sql_key_info {
 	sql *db;
+	/** Informations about all field types and key parts. */
+	struct space_info *info;
 	/**
 	 * Key definition created from this object,
 	 * see sql_key_info_to_key_def().
@@ -4055,6 +4057,66 @@ 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 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;
+	/** 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;
+	/** Sort order of index. */
+	enum sort_order sort_order;
+};
+
+/**
+ * Allocate and initialize with default values a structure that will be used to
+ * store information about ephemeral space field types and key parts.
+ */
+struct 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.
+ */
+void
+sql_space_info_from_space_def(struct space_info *info,
+			      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.
+ */
+void
+sql_space_info_from_index_def(struct space_info *info,
+			      const struct index_def *def);
+
+/**
+ * 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_from_info(const struct 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..8eab8a3df 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -226,9 +226,21 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	iEph = pParse->nTab++;
 	sqlVdbeAddOp2(v, OP_Null, 0, iPk);
 
+	struct space_info *info = sql_space_info_new(pk_part_count, 0);
+	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,
+				     pk_part_count, 0, (char *)info,
+				     P4_DYNAMIC);
+	assert(space->index_count > 0 || is_view);
+	if (is_view)
+		sql_space_info_from_space_def(info, def);
+	else
+		sql_space_info_from_index_def(info, pPk->def);
+
 	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..14caa6f82 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;
 }
@@ -2392,10 +2390,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);
+	struct space_info *info = pOp->p4type == P4_KEYINFO ?
+				  pOp->p4.key_info->info : pOp->p4.space_info;
+	assert(info != NULL);
+	struct space *space = sql_ephemeral_space_new_from_info(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..505892861 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 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..31490a9b2 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,15 +919,16 @@ 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 space_info *info = sql_space_info_new(nKeyCol + 1, 0);
+	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);
+		      (char *)info, P4_DYNAMIC);
+	sql_space_info_from_index_def(info, idx_def);
+	info->types[nKeyCol] = FIELD_TYPE_INTEGER;
 	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..a8a1893d3 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 space_info *info =
+				sql_space_info_new(pk_part_count, 0);
+			if (info == NULL) {
+				pParse->is_aborted = true;
+				return notReady;
+			}
+			sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set,
+				      pk_part_count, 0, (char *)info,
+				      P4_DYNAMIC);
+			struct index_def *index_def = space->index[0]->def;
+			sql_space_info_from_index_def(info, index_def);
 			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);

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-12 21:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 14:14 [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces Mergen Imeev via Tarantool-patches
2021-07-23 23:06 ` Vladislav Shpilevoy via Tarantool-patches
2021-08-10 14:32   ` Mergen Imeev via Tarantool-patches
2021-08-12 18:32     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-12 18:56       ` Safin Timur via Tarantool-patches
2021-08-12 21:49       ` Mergen Imeev via Tarantool-patches

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