* [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields
@ 2021-08-13 3:09 Mergen Imeev via Tarantool-patches
2021-08-13 15:12 ` Vladimir Davydov via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-13 3:09 UTC (permalink / raw)
To: vdavydov; +Cc: tarantool-patches, Mergen Imeev
From: Mergen Imeev <imeevma@gmail.com>
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
---
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 | 215 ++++++++++++++++-----------
src/box/sql/delete.c | 34 ++---
src/box/sql/expr.c | 18 +--
src/box/sql/insert.c | 61 +++++---
src/box/sql/select.c | 290 +++++++++++++++++++++++++++++++------
src/box/sql/sqlInt.h | 62 ++++++++
src/box/sql/tarantoolInt.h | 15 --
src/box/sql/update.c | 16 +-
src/box/sql/vdbe.c | 13 +-
src/box/sql/vdbe.h | 13 +-
src/box/sql/vdbeaux.c | 12 --
src/box/sql/where.c | 9 +-
src/box/sql/wherecode.c | 14 +-
13 files changed, 543 insertions(+), 229 deletions(-)
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);
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields
2021-08-13 3:09 [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields Mergen Imeev via Tarantool-patches
@ 2021-08-13 15:12 ` Vladimir Davydov via Tarantool-patches
2021-08-15 13:26 ` Mergen Imeev via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Davydov via Tarantool-patches @ 2021-08-13 15:12 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches, Mergen Imeev
On Fri, Aug 13, 2021 at 06:09:19AM +0300, imeevma@tarantool.org wrote:
> diff --git a/src/box/sql.c b/src/box/sql.c
> index c805a1e5c..2b95aed8d 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -243,114 +243,163 @@ tarantoolsqlCount(struct BtCursor *pCur)
> return index_count(pCur->index, pCur->iter_type, NULL, 0);
> }
>
> -struct space *
> -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> +struct space_info *
> +sql_space_info_new(uint32_t field_count, uint32_t part_count)
> {
> - struct key_def *def = NULL;
> - uint32_t part_count = field_count;
> - if (key_info != NULL) {
> - def = sql_key_info_to_key_def(key_info);
> - if (def == NULL)
> - return NULL;
> - /*
> - * In case is_pk_rowid is true we can use rowid
> - * as the only part of the key.
> - */
> - if (key_info->is_pk_rowid)
> - part_count = 1;
> + assert(field_count > 0);
> + uint32_t info_size = sizeof(struct space_info);
> + uint32_t field_size = field_count * sizeof(enum field_type);
> + uint32_t colls_size = field_count * sizeof(uint32_t);
> + uint32_t parts_size = part_count * sizeof(uint32_t);
> + uint32_t size = info_size + field_size + colls_size + parts_size;
> +
> + struct space_info *info = sqlDbMallocRawNN(sql_get(), size);
> + if (info == NULL) {
> + diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
> + return NULL;
> }
> + info->types = (enum field_type *)((char *)info + info_size);
> + info->coll_ids = (uint32_t *)((char *)info->types + field_size);
> + info->parts = part_count == 0 ? NULL :
> + (uint32_t *)((char *)info->coll_ids + colls_size);
> + info->field_count = field_count;
> + info->part_count = part_count;
> + info->sort_order = SORT_ORDER_ASC;
>
> - struct region *region = &fiber()->gc;
> + for (uint32_t i = 0; i < field_count; ++i) {
> + info->types[i] = FIELD_TYPE_SCALAR;
> + info->coll_ids[i] = COLL_NONE;
> + }
> + for (uint32_t i = 0; i < part_count; ++i)
> + info->parts[i] = i;
> + return info;
> +}
> +
> +void
> +sql_space_info_from_space_def(struct space_info *info,
> + const struct space_def *def)
Separating allocation from initialization only complicates the protocol:
- The caller must ensure that the arguments passed to alloc and init
are compatible.
- There's a window between alloc and init when the object is unusable.
Please don't:
struct sql_space_info *
sql_space_info_new_from_space_def(...);
struct sql_space_info *
sql_space_info_new_from_index_def(...);
> +{
> + 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,
> +};
Please move this definition to the function that uses it.
> +
> +struct space *
> +sql_ephemeral_space_new_from_info(const struct space_info *info)
struct space *
sql_ephemeral_space_new(const struct sql_space_info *info);
> +{
> + uint32_t field_count = info->field_count;
> + uint32_t part_count = info->parts == NULL ? field_count :
> + info->part_count;
> + uint32_t parts_indent = field_count * sizeof(struct field_def);
> + uint32_t names_indent = part_count * sizeof(struct key_part_def) +
> + parts_indent;
> + 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) {
uint32_t j = info->parts != NULL ? info->parts[i] : i;
parts[i].fieldno = 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/select.c b/src/box/sql/select.c
> index b9107fccc..a3e551017 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -5879,6 +6047,23 @@ sqlSelect(Parse * pParse, /* The parser context */
> sSort.reg_eph,
> nCols,
> 0, (char *)key_info, P4_KEYINFO);
> + /*
> + * We also should define space_info in case this opcode will not
> + * be converted to SorterOpen.
> + */
> + uint32_t part_count = sSort.pOrderBy->nExpr + 1;
> + struct space_info *info = sql_space_info_new(nCols, part_count);
> + if (info == NULL) {
> + pParse->is_aborted = true;
> + goto select_end;
> + }
> + key_info->info = info;
> + if (space_info_for_sorting(info, pParse, sSort.pOrderBy,
> + pEList) != 0) {
> + pParse->is_aborted = true;
> + goto select_end;
> + }
> +
I don't understand why you can't pass sql_space_info instead of
sql_key_info. AFAIU, you don't need key_info here at all anymore.
Anyway, keeping a pointer to space_info in key_info looks bad.
Can you avoid that?
> sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
> VdbeComment((v, "Sort table"));
> } else {
> 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
> @@ -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 {
sql_space_info
> + /** Field types of all fields of ephemeral space. */
> + enum field_type *types;
> + /** Collation ids of all fields of ephemeral space. */
> + uint32_t *coll_ids;
> + /**
> + * Fieldno key parts of the ephemeral space. If NULL, then the index
> + * consists of all fields in sequential order.
> + */
> + uint32_t *parts;
> + /** 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;
> +};
> 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
> @@ -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);
You don't use p2 anymore. Do you still need to pass it?
> + 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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields
2021-08-13 15:12 ` Vladimir Davydov via Tarantool-patches
@ 2021-08-15 13:26 ` Mergen Imeev via Tarantool-patches
2021-08-16 8:57 ` Vladimir Davydov via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-15 13:26 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches, Mergen Imeev
Hi! Thank you for the review! In general, I want to say that I am very sorry for
such low quality of this code. I couldn't come to a simpler way to solve the
issue. If there will be some time, I will rework ephemeral spaces and sorter.
Now I cannot do this since it is a lot of work. I also cannot drop this patch
since it will lead to a lot of problems.
I fixed most of your comments. See my answers, diff and new patch below.
On Fri, Aug 13, 2021 at 06:12:29PM +0300, Vladimir Davydov wrote:
> On Fri, Aug 13, 2021 at 06:09:19AM +0300, imeevma@tarantool.org wrote:
> > diff --git a/src/box/sql.c b/src/box/sql.c
> > index c805a1e5c..2b95aed8d 100644
> > --- a/src/box/sql.c
> > +++ b/src/box/sql.c
> > @@ -243,114 +243,163 @@ tarantoolsqlCount(struct BtCursor *pCur)
> > return index_count(pCur->index, pCur->iter_type, NULL, 0);
> > }
> >
> > -struct space *
> > -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> > +struct space_info *
> > +sql_space_info_new(uint32_t field_count, uint32_t part_count)
> > {
> > - struct key_def *def = NULL;
> > - uint32_t part_count = field_count;
> > - if (key_info != NULL) {
> > - def = sql_key_info_to_key_def(key_info);
> > - if (def == NULL)
> > - return NULL;
> > - /*
> > - * In case is_pk_rowid is true we can use rowid
> > - * as the only part of the key.
> > - */
> > - if (key_info->is_pk_rowid)
> > - part_count = 1;
> > + assert(field_count > 0);
> > + uint32_t info_size = sizeof(struct space_info);
> > + uint32_t field_size = field_count * sizeof(enum field_type);
> > + uint32_t colls_size = field_count * sizeof(uint32_t);
> > + uint32_t parts_size = part_count * sizeof(uint32_t);
> > + uint32_t size = info_size + field_size + colls_size + parts_size;
> > +
> > + struct space_info *info = sqlDbMallocRawNN(sql_get(), size);
> > + if (info == NULL) {
> > + diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
> > + return NULL;
> > }
> > + info->types = (enum field_type *)((char *)info + info_size);
> > + info->coll_ids = (uint32_t *)((char *)info->types + field_size);
> > + info->parts = part_count == 0 ? NULL :
> > + (uint32_t *)((char *)info->coll_ids + colls_size);
> > + info->field_count = field_count;
> > + info->part_count = part_count;
> > + info->sort_order = SORT_ORDER_ASC;
> >
> > - struct region *region = &fiber()->gc;
> > + for (uint32_t i = 0; i < field_count; ++i) {
> > + info->types[i] = FIELD_TYPE_SCALAR;
> > + info->coll_ids[i] = COLL_NONE;
> > + }
> > + for (uint32_t i = 0; i < part_count; ++i)
> > + info->parts[i] = i;
> > + return info;
> > +}
> > +
> > +void
> > +sql_space_info_from_space_def(struct space_info *info,
> > + const struct space_def *def)
>
> Separating allocation from initialization only complicates the protocol:
> - The caller must ensure that the arguments passed to alloc and init
> are compatible.
> - There's a window between alloc and init when the object is unusable.
>
> Please don't:
>
> struct sql_space_info *
> sql_space_info_new_from_space_def(...);
>
> struct sql_space_info *
> sql_space_info_new_from_index_def(...);
>
Fixed. There is still some places that cannot be fixed for now, since
sql_space_info may be modified during execution of some code.
> > +{
> > + 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,
> > +};
>
> Please move this definition to the function that uses it.
>
Fixed.
> > +
> > +struct space *
> > +sql_ephemeral_space_new_from_info(const struct space_info *info)
>
> struct space *
> sql_ephemeral_space_new(const struct sql_space_info *info);
>
I would like to do this, but we already have a function with such name. More
than that, this functions actually have no connection to ephemeral spaces,
the "spaces" it creates are only needed for creation of normal spaces, i.e.
it actually creates analogues of space_def. Not sure why space_def cannot be
used there.
> > +{
> > + 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) {
> uint32_t j = info->parts != NULL ? info->parts[i] : i;
> parts[i].fieldno = j;
> ...
> }
>
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;
> > - }
> > - 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/select.c b/src/box/sql/select.c
> > index b9107fccc..a3e551017 100644
> > --- a/src/box/sql/select.c
> > +++ b/src/box/sql/select.c
> > @@ -5879,6 +6047,23 @@ sqlSelect(Parse * pParse, /* The parser context */
> > sSort.reg_eph,
> > nCols,
> > 0, (char *)key_info, P4_KEYINFO);
> > + /*
> > + * We also should define space_info in case this opcode will not
> > + * be converted to SorterOpen.
> > + */
> > + uint32_t part_count = sSort.pOrderBy->nExpr + 1;
> > + struct space_info *info = sql_space_info_new(nCols, part_count);
> > + if (info == NULL) {
> > + pParse->is_aborted = true;
> > + goto select_end;
> > + }
> > + key_info->info = info;
> > + if (space_info_for_sorting(info, pParse, sSort.pOrderBy,
> > + pEList) != 0) {
> > + pParse->is_aborted = true;
> > + goto select_end;
> > + }
> > +
>
> I don't understand why you can't pass sql_space_info instead of
> sql_key_info. AFAIU, you don't need key_info here at all anymore.
> Anyway, keeping a pointer to space_info in key_info looks bad.
> Can you avoid that?
>
The problem here is that OpenTEphemeral opcode can be replaced by SorterOpen
opcode. Also, its key_info could be moved to OP_Compare opcode. Actually, I
made a patch which also replaces sql_key_info by space_info for these two
opcodes, but space_info become too complicated. I decided to leave it for now.
I actually think that we should drop sorter and OP_Compare, but not sure that
we should do it now, before release. I will think of it a bit later, if I will
be given some time to rework this.
> > sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
> > VdbeComment((v, "Sort table"));
> > } else {
> > 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
> > @@ -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 {
>
> sql_space_info
>
Fixed.
> > + /** 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;
> > +};
> > 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
> > @@ -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);
>
> You don't use p2 anymore. Do you still need to pass it?
>
Fixed in most cases. The same case when ephemeral space can be replaced by
SorterOpen still needs p2 though.
> > + 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:
diff --git a/src/box/sql.c b/src/box/sql.c
index 2b95aed8d..5708db6b6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -243,17 +243,17 @@ tarantoolsqlCount(struct BtCursor *pCur)
return index_count(pCur->index, pCur->iter_type, NULL, 0);
}
-struct space_info *
+struct sql_space_info *
sql_space_info_new(uint32_t field_count, uint32_t part_count)
{
assert(field_count > 0);
- uint32_t info_size = sizeof(struct space_info);
+ uint32_t info_size = sizeof(struct sql_space_info);
uint32_t field_size = field_count * sizeof(enum field_type);
uint32_t colls_size = field_count * sizeof(uint32_t);
uint32_t parts_size = part_count * sizeof(uint32_t);
uint32_t size = info_size + field_size + colls_size + parts_size;
- struct space_info *info = sqlDbMallocRawNN(sql_get(), size);
+ struct sql_space_info *info = sqlDbMallocRawNN(sql_get(), size);
if (info == NULL) {
diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
return NULL;
@@ -275,44 +275,42 @@ sql_space_info_new(uint32_t field_count, uint32_t part_count)
return info;
}
-void
-sql_space_info_from_space_def(struct space_info *info,
- const struct space_def *def)
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def)
{
- assert(info->field_count == def->field_count + 1);
- assert(info->part_count == 0);
+ uint32_t field_count = def->field_count + 1;
+ struct sql_space_info *info = sql_space_info_new(field_count, 0);
+ if (info == NULL)
+ return NULL;
for (uint32_t i = 0; i < def->field_count; ++i) {
info->types[i] = def->fields[i].type;
info->coll_ids[i] = def->fields[i].coll_id;
}
/* Add one more field for rowid. */
info->types[def->field_count] = FIELD_TYPE_INTEGER;
- info->coll_ids[def->field_count] = COLL_NONE;
+ return info;
}
-void
-sql_space_info_from_index_def(struct space_info *info,
- const struct index_def *def)
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid)
{
- assert(info->field_count >= def->key_def->part_count);
- assert(info->part_count == 0);
+ uint32_t field_count = def->key_def->part_count;
+ if (has_rowid)
+ ++field_count;
+ struct sql_space_info *info = sql_space_info_new(field_count, 0);
+ if (info == NULL)
+ return NULL;
for (uint32_t i = 0; i < def->key_def->part_count; ++i) {
info->types[i] = def->key_def->parts[i].type;
info->coll_ids[i] = def->key_def->parts[i].coll_id;
}
+ if (has_rowid)
+ info->types[def->key_def->part_count] = FIELD_TYPE_INTEGER;
+ return info;
}
-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,
-};
-
struct space *
-sql_ephemeral_space_new_from_info(const struct space_info *info)
+sql_ephemeral_space_new_from_info(const struct sql_space_info *info)
{
uint32_t field_count = info->field_count;
uint32_t part_count = info->parts == NULL ? field_count :
@@ -320,7 +318,13 @@ sql_ephemeral_space_new_from_info(const struct space_info *info)
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;
+ /*
+ * Name of the fields will be "_COLUMN_1", "_COLUMN_2" and so on. Due to
+ * this, length of each name is no more than 19 == strlen("_COLUMN_")
+ * plus length of UINT32_MAX turned to string, which is 10 and plus 1
+ * for '\0'.
+ */
+ uint32_t size = names_indent + field_count * 19;
struct region *region = &fiber()->gc;
size_t svp = region_used(region);
@@ -337,8 +341,9 @@ sql_ephemeral_space_new_from_info(const struct space_info *info)
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].name = names;
+ sprintf(names, "_COLUMN_%d", i);
+ names += strlen(fields[i].name) + 1;
fields[i].is_nullable = true;
fields[i].nullable_action = ON_CONFLICT_ACTION_NONE;
fields[i].default_value = NULL;
@@ -346,29 +351,16 @@ sql_ephemeral_space_new_from_info(const struct space_info *info)
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) {
+ uint32_t j = info->parts == NULL ? i : info->parts[i];
+ parts[i].fieldno = j;
+ parts[i].nullable_action = ON_CONFLICT_ACTION_NONE;
+ parts[i].is_nullable = true;
+ parts[i].exclude_null = false;
+ parts[i].sort_order = 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/delete.c b/src/box/sql/delete.c
index 9d5a63fc5..b82a6874e 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -230,14 +230,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
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);
+ struct sql_space_info *info;
if (is_view) {
/*
* At this stage SELECT is already materialized
@@ -257,12 +250,20 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* account that id field as well. That's why pk_len
* has one field more than view format.
*/
- sql_space_info_from_space_def(info, space->def);
+ info = sql_space_info_new_from_space_def(space->def);
} else {
assert(space->index_count > 0);
struct index_def *index_def = space->index[0]->def;
- sql_space_info_from_index_def(info, index_def);
+ info = sql_space_info_new_from_index_def(index_def,
+ false);
}
+ if (info == NULL) {
+ parse->is_aborted = true;
+ goto delete_from_cleanup;
+ }
+ parse->nMem += pk_len;
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+ P4_DYNAMIC);
/* Construct a query to find the primary key for
* every row to be deleted, based on the WHERE
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index c67a7091c..8902c648f 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2760,10 +2760,11 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
*/
pExpr->iTable = pParse->nTab++;
int reg_eph = ++pParse->nMem;
- struct space_info *info = sql_space_info_new(nVal, 0);
+ struct sql_space_info *info =
+ sql_space_info_new(nVal, 0);
if (info == NULL)
return 0;
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nVal, 0,
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
(char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, pExpr->iTable, 0,
reg_eph);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 80579c1d2..cc7b6b0e3 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -60,29 +60,25 @@ 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,
+static struct sql_space_info *
+sql_space_info_from_id_list(const struct space_def *def, int len,
const struct IdList *list)
{
- int n = info->field_count - 1;
+ struct sql_space_info *info = sql_space_info_new(len + 1, 1);
+ if (info == NULL)
+ return NULL;
+ assert(len > 0 && len <= (int)def->field_count);
struct field_def *fields = def->fields;
- 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;
- }
+ for (int i = 0; i < len; ++i) {
+ int j = list == NULL ? i : list->a[i].idx;
+ info->types[i] = fields[j].type;
+ info->coll_ids[i] = fields[j].coll_id;
}
/* Add one more field for rowid. */
- info->types[n] = FIELD_TYPE_INTEGER;
+ info->types[len] = FIELD_TYPE_INTEGER;
/* Set rowid as the only part of primary index. */
- info->parts[0] = n;
+ info->parts[0] = len;
+ return info;
}
/**
@@ -474,16 +470,15 @@ sqlInsert(Parse * pParse, /* Parser context */
* rowid. Since each rowid is unique, we do not need any
* other parts.
*/
- struct space_info *info =
- sql_space_info_new(nColumn + 1, 1);
+ struct sql_space_info *info =
+ sql_space_info_from_id_list(space_def, nColumn,
+ pColumn);
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);
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
addrL = sqlVdbeAddOp1(v, OP_Yield, dest.iSDParm);
VdbeCoverage(v);
sqlVdbeAddOp2(v, OP_NextIdEphemeral, reg_eph,
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index a3e551017..4aed24822 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -102,16 +102,20 @@ struct SortCtx {
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)
+static struct sql_space_info *
+sql_space_info_new_from_expr_list(struct Parse *parser, struct ExprList *list,
+ bool has_rowid)
{
- assert((int)info->field_count >= list->nExpr - start);
- int n = list->nExpr - start;
- for (int i = 0; i < n; ++i) {
+ int n = list->nExpr;
+ if (has_rowid)
+ ++n;
+ struct sql_space_info *info = sql_space_info_new(n, 0);
+ if (info == NULL)
+ return NULL;
+ for (int i = 0; i < list->nExpr; ++i) {
bool b;
struct coll *coll;
- struct Expr *expr = list->a[i + start].pExpr;
+ 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
@@ -122,23 +126,26 @@ space_info_from_expr_list(struct space_info *info, struct Parse *parser,
type = FIELD_TYPE_SCALAR;
uint32_t coll_id;
if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
- return -1;
+ return NULL;
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;
+ info->types[list->nExpr] = FIELD_TYPE_INTEGER;
+ return info;
}
-static int
-space_info_from_order_by(struct space_info *info, struct Parse *parser,
- struct Select *select, struct ExprList *list)
+static struct sql_space_info *
+sql_space_info_new_from_order_by(struct Parse *parser, struct Select *select,
+ struct ExprList *order_by)
{
- for (int i = 0; i < list->nExpr; ++i) {
- bool b;
- struct Expr *expr = list->a[i].pExpr;
+ int n = order_by->nExpr + 2;
+ struct sql_space_info *info = sql_space_info_new(n, 0);
+ if (info == NULL)
+ return NULL;
+ for (int i = 0; i < order_by->nExpr; ++i) {
+ struct Expr *expr = order_by->a[i].pExpr;
enum field_type type = sql_expr_type(expr);
/*
* Type ANY could mean that field was unresolved. We have no way
@@ -150,34 +157,76 @@ space_info_from_order_by(struct space_info *info, struct Parse *parser,
info->types[i] = type;
uint32_t *id = &info->coll_ids[i];
if ((expr->flags & EP_Collate) != 0) {
+ bool b;
struct coll *coll;
if (sql_expr_coll(parser, expr, &b, id, &coll) != 0)
- return -1;
+ return NULL;
continue;
}
- uint32_t fieldno = list->a[i].u.x.iOrderByCol - 1;
+ uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
info->coll_ids[i] = multi_select_coll_seq(parser, select,
fieldno);
if (info->coll_ids[i] != COLL_NONE) {
const char *name = coll_by_id(info->coll_ids[i])->name;
- list->a[i].pExpr =
+ order_by->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;
+ info->sort_order = order_by->a[0].sort_order;
+ info->types[order_by->nExpr] = FIELD_TYPE_INTEGER;
+ info->types[order_by->nExpr + 1] = FIELD_TYPE_VARBINARY;
+ return info;
}
-static int
-space_info_for_sorting(struct space_info *info, struct Parse *parser,
- struct ExprList *order_by, struct ExprList *list)
+static struct sql_space_info *
+sql_space_info_new_for_sorting(struct Parse *parser, struct ExprList *order_by,
+ struct ExprList *list, int start, bool has_rowid)
{
- if (space_info_from_expr_list(info, parser, order_by, 0, true) != 0)
- return -1;
- uint32_t part_count = order_by->nExpr + 1;
+ /*
+ * Index consist of fields that were not included into index plus rowid,
+ * is has_rowid is TRUE.
+ */
+ uint32_t part_count = order_by->nExpr - start;
+ if (has_rowid)
+ ++part_count;
+ /*
+ * Number of fields is number of parts in index plus number of fields
+ * that is not appear in ORDER BY, but were in SELECT.
+ */
+ uint32_t field_count = part_count;
+ /* If iOrderByCol != 0 than fields appear in ORDER BY. */
+ for (int i = 0; i < list->nExpr; ++i)
+ field_count += list->a[i].u.x.iOrderByCol == 0 ? 1 : 0;
+
+ struct sql_space_info *info =
+ sql_space_info_new(field_count, part_count);
+ if (info == NULL)
+ return NULL;
+ int k;
+ for (k = 0; k < order_by->nExpr - start; ++k) {
+ bool b;
+ struct coll *coll;
+ struct Expr *expr = order_by->a[k + start].pExpr;
+ enum field_type type = sql_expr_type(expr);
+ /*
+ * Type ANY could mean that field was unresolved. We have no way
+ * but to set it as SCALAR, however this could lead to
+ * unexpected change of type.
+ */
+ if (type == FIELD_TYPE_ANY)
+ type = FIELD_TYPE_SCALAR;
+ uint32_t coll_id;
+ if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
+ return NULL;
+ info->types[k] = type;
+ info->coll_ids[k] = coll_id;
+ }
+ if (has_rowid)
+ info->types[k++] = FIELD_TYPE_INTEGER;
+ assert(k == (int)part_count);
for (int i = 0; i < list->nExpr; ++i) {
+ if (list->a[i].u.x.iOrderByCol != 0)
+ continue;
bool b;
struct Expr *expr = list->a[i].pExpr;
enum field_type type = sql_expr_type(expr);
@@ -186,12 +235,14 @@ space_info_for_sorting(struct space_info *info, struct Parse *parser,
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 NULL;
+ info->types[k] = type;
+ info->coll_ids[k] = id;
+ ++k;
}
- return 0;
+ assert(k == (int)field_count);
+ info->sort_order = order_by->a[0].sort_order;
+ return info;
}
/*
@@ -926,20 +977,17 @@ pushOntoSorter(Parse * pParse, /* Parser context */
pSort->pOrderBy,
nOBSat);
} else {
- struct space_info *info =
- sql_space_info_new(pOp->p2, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_for_sorting(pParse,
+ pSort->pOrderBy,
+ pSelect->pEList,
+ nOBSat, bSeq);
if (info == NULL) {
pParse->is_aborted = true;
return;
}
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);
@@ -1161,7 +1209,7 @@ selectInnerLoop(Parse * pParse, /* The parser context */
uint32_t excess_field_count = 0;
struct VdbeOp *op = sqlVdbeGetOp(v,
pSort->addrSortIndex);
- struct space_info *info;
+ struct sql_space_info *info;
if (op->p4type == P4_KEYINFO)
info = op->p4.key_info->info;
else
@@ -2586,33 +2634,26 @@ 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);
+ struct sql_space_info *info;
if (pOrderBy) {
VdbeComment((v, "Orderby table"));
- if (space_info_from_order_by(info, pParse, p, pOrderBy) != 0) {
- pParse->is_aborted = true;
- goto end_of_recursive_query;
- }
+ info = sql_space_info_new_from_order_by(pParse, p, pOrderBy);
destQueue.pOrderBy = pOrderBy;
} else {
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;
- }
+ info = sql_space_info_new_from_expr_list(pParse, p->pEList,
+ true);
+ }
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto end_of_recursive_query;
}
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, 0, 0, (char *)info,
+ P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
if (iDistinct) {
p->addrOpenEphm[0] =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_dist, 1);
+ sqlVdbeAddOp1(v, OP_OpenTEphemeral, reg_dist);
sqlVdbeAddOp3(v, OP_IteratorOpen, iDistinct, 0, reg_dist);
p->selFlags |= SF_UsesEphemeral;
VdbeComment((v, "Distinct table"));
@@ -2816,21 +2857,16 @@ multiSelect(Parse * pParse, /* Parsing context */
*/
if (dest.eDest == SRT_EphemTab) {
assert(p->pEList);
- int nCols = p->pEList->nExpr;
- struct space_info *info = sql_space_info_new(nCols + 1, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, p->pEList,
+ true);
if (info == NULL) {
pParse->is_aborted = true;
rc = 1;
goto multi_select_end;
}
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, 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;
- }
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
VdbeComment((v, "Destination temp"));
dest.eDest = SRT_Table;
@@ -2946,10 +2982,9 @@ multiSelect(Parse * pParse, /* Parsing context */
unionTab = pParse->nTab++;
reg_union = ++pParse->nMem;
assert(p->pOrderBy == 0);
- addr =
- sqlVdbeAddOp2(v,
- OP_OpenTEphemeral,
- reg_union, 0);
+ addr = sqlVdbeAddOp1(v,
+ OP_OpenTEphemeral,
+ reg_union);
sqlVdbeAddOp3(v, OP_IteratorOpen, unionTab, 0, reg_union);
assert(p->addrOpenEphm[0] == -1);
p->addrOpenEphm[0] = addr;
@@ -3062,9 +3097,8 @@ multiSelect(Parse * pParse, /* Parsing context */
reg_eph2 = ++pParse->nMem;
assert(p->pOrderBy == 0);
- addr =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph1,
- 0);
+ addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+ reg_eph1);
sqlVdbeAddOp3(v, OP_IteratorOpen, tab1, 0, reg_eph1);
assert(p->addrOpenEphm[0] == -1);
p->addrOpenEphm[0] = addr;
@@ -3084,9 +3118,8 @@ multiSelect(Parse * pParse, /* Parsing context */
/* Code the current SELECT into temporary table "tab2"
*/
- addr =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph2,
- 0);
+ addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+ reg_eph2);
sqlVdbeAddOp3(v, OP_IteratorOpen, tab2, 0, reg_eph2);
assert(p->addrOpenEphm[1] == -1);
p->addrOpenEphm[1] = addr;
@@ -3158,7 +3191,7 @@ multiSelect(Parse * pParse, /* Parsing context */
if (p->selFlags & SF_UsesEphemeral) {
assert(p->pNext == NULL);
int nCol = p->pEList->nExpr;
- struct space_info *info = sql_space_info_new(nCol, 0);
+ struct sql_space_info *info = sql_space_info_new(nCol, 0);
if (info == NULL) {
pParse->is_aborted = true;
goto multi_select_end;
@@ -5509,21 +5542,17 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo)
return;
}
assert(pE->x.pList->nExpr == 1);
- struct space_info *info = sql_space_info_new(1, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse,
+ pE->x.pList,
+ false);
if (info == NULL) {
pParse->is_aborted = true;
pFunc->iDistinct = -1;
return;
}
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1,
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 0,
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);
}
@@ -6042,27 +6071,25 @@ sqlSelect(Parse * pParse, /* The parser context */
if (key_info->parts[0].sort_order == SORT_ORDER_DESC) {
sSort.sortFlags |= SORTFLAG_DESC;
}
+ /*
+ * We need to defined both sql_key_info and sql_space_info since
+ * this opcode may change to OP_SorterOpen, which uses
+ * sql_key_info. This is also the reason why we should give p2
+ * for this opcode.
+ */
sSort.addrSortIndex =
sqlVdbeAddOp4(v, OP_OpenTEphemeral,
sSort.reg_eph,
nCols,
0, (char *)key_info, P4_KEYINFO);
- /*
- * 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);
+ struct sql_space_info *info =
+ sql_space_info_new_for_sorting(pParse, sSort.pOrderBy,
+ pEList, 0, true);
if (info == NULL) {
pParse->is_aborted = true;
goto select_end;
}
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"));
@@ -6073,19 +6100,14 @@ sqlSelect(Parse * pParse, /* The parser context */
/* If the output is destined for a temporary table, open that table.
*/
if (pDest->eDest == SRT_EphemTab) {
- uint32_t cols = pEList->nExpr + 1;
- struct space_info *info = sql_space_info_new(cols, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, pEList, true);
if (info == NULL) {
pParse->is_aborted = true;
goto select_end;
}
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, cols, 0,
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, 0, 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);
@@ -6111,20 +6133,16 @@ sqlSelect(Parse * pParse, /* The parser context */
if (p->selFlags & SF_Distinct) {
sDistinct.cur_eph = pParse->nTab++;
sDistinct.reg_eph = ++pParse->nMem;
- uint32_t cols = pEList->nExpr;
- struct space_info *info = sql_space_info_new(cols, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, pEList,
+ false);
if (info == NULL) {
pParse->is_aborted = true;
goto select_end;
}
sDistinct.addrTnct = sqlVdbeAddOp4(v, OP_OpenTEphemeral,
- sDistinct.reg_eph, cols, 0,
+ sDistinct.reg_eph, 0, 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 d854562d0..2564eda8a 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4007,7 +4007,7 @@ sql_analysis_load(struct sql *db);
struct sql_key_info {
sql *db;
/** Informations about all field types and key parts. */
- struct space_info *info;
+ struct sql_space_info *info;
/**
* Key definition created from this object,
* see sql_key_info_to_key_def().
@@ -4061,7 +4061,7 @@ 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 {
+struct sql_space_info {
/** Field types of all fields of ephemeral space. */
enum field_type *types;
/** Collation ids of all fields of ephemeral space. */
@@ -4086,7 +4086,7 @@ struct space_info {
* 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 *
+struct sql_space_info *
sql_space_info_new(uint32_t field_count, uint32_t part_count);
/**
@@ -4096,9 +4096,8 @@ sql_space_info_new(uint32_t field_count, uint32_t part_count);
* 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);
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def);
/**
* Initialize the field types and key parts of space_info with index_def.
@@ -4106,16 +4105,15 @@ sql_space_info_from_space_def(struct space_info *info,
* 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);
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid);
/**
* Allocate and initialize an ephemeral space. Information about field types and
* key parts is taken from the space_info structure.
*/
struct space *
-sql_ephemeral_space_new_from_info(const struct space_info *info);
+sql_ephemeral_space_new_from_info(const struct sql_space_info *info);
/**
* Check if the function implements LIKE-style comparison & if it
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 8eab8a3df..b4827f242 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -226,20 +226,19 @@ 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);
+ struct sql_space_info *info;
+ assert(space->index_count > 0 || is_view);
+ if (is_view)
+ info = sql_space_info_new_from_space_def(def);
+ else
+ info = sql_space_info_new_from_index_def(pPk->def, false);
if (info == NULL) {
pParse->is_aborted = true;
goto update_cleanup;
}
/* Address of the OpenEphemeral instruction. */
- int addrOpen = 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);
+ int addrOpen = sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
pWInfo = sqlWhereBegin(pParse, pTabList, pWhere, 0, 0,
WHERE_ONEPASS_DESIRED, pk_cursor);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 14caa6f82..0facd75c7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2378,10 +2378,9 @@ open_cursor_set_hints:
}
/**
- * Opcode: OpenTEphemeral P1 P2 * P4 *
+ * Opcode: OpenTEphemeral P1 * * P4 *
* Synopsis:
* @param P1 register, where pointer to new space is stored.
- * @param P2 number of columns in a new table.
* @param P4 key def for new table, NULL is allowed.
*
* This opcode creates Tarantool's ephemeral table and stores pointer
@@ -2389,10 +2388,10 @@ open_cursor_set_hints:
*/
case OP_OpenTEphemeral: {
assert(pOp->p1 >= 0);
- assert(pOp->p2 > 0);
- struct space_info *info = pOp->p4type == P4_KEYINFO ?
- pOp->p4.key_info->info : pOp->p4.space_info;
+ struct sql_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);
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 505892861..e40a1a0b3 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -96,7 +96,7 @@ struct VdbeOp {
/**
* Information about ephemeral space field types and key parts.
*/
- struct space_info *space_info;
+ struct sql_space_info *space_info;
} p4;
#ifdef SQL_ENABLE_EXPLAIN_COMMENTS
char *zComment; /* Comment to improve readability */
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 31490a9b2..d8d23161b 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,16 +919,15 @@ constructAutomaticIndex(Parse * pParse, /* The parsing context */
/* Create the automatic index */
assert(pLevel->iIdxCur >= 0);
pLevel->iIdxCur = pParse->nTab++;
- struct space_info *info = sql_space_info_new(nKeyCol + 1, 0);
+ struct sql_space_info *info = sql_space_info_new_from_index_def(idx_def,
+ true);
if (info == NULL) {
pParse->is_aborted = true;
return;
}
int reg_eph = sqlGetTempReg(pParse);
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nKeyCol + 1, 0,
- (char *)info, P4_DYNAMIC);
- sql_space_info_from_index_def(info, idx_def);
- info->types[nKeyCol] = FIELD_TYPE_INTEGER;
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+ P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, pLevel->iIdxCur, 0, reg_eph);
VdbeComment((v, "for %s", space->def->name));
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index a8a1893d3..0d0cb054d 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1134,8 +1134,10 @@ 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;
- struct space_info *info =
- sql_space_info_new(pk_part_count, 0);
+ struct index_def *index_def = space->index[0]->def;
+ struct sql_space_info *info =
+ sql_space_info_new_from_index_def(index_def,
+ false);
if (info == NULL) {
pParse->is_aborted = true;
return notReady;
@@ -1143,8 +1145,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W
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);
regPk = ++pParse->nMem;
New patch:
commit c91b300a91366a620f8afc22fc86ae4b09bd176c
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..5708db6b6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -243,114 +243,155 @@ tarantoolsqlCount(struct BtCursor *pCur)
return index_count(pCur->index, pCur->iter_type, NULL, 0);
}
-struct space *
-sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
+struct sql_space_info *
+sql_space_info_new(uint32_t field_count, uint32_t part_count)
{
- struct key_def *def = NULL;
- uint32_t part_count = field_count;
- if (key_info != NULL) {
- def = sql_key_info_to_key_def(key_info);
- if (def == NULL)
- return NULL;
- /*
- * In case is_pk_rowid is true we can use rowid
- * as the only part of the key.
- */
- if (key_info->is_pk_rowid)
- part_count = 1;
+ assert(field_count > 0);
+ uint32_t info_size = sizeof(struct sql_space_info);
+ uint32_t field_size = field_count * sizeof(enum field_type);
+ uint32_t colls_size = field_count * sizeof(uint32_t);
+ uint32_t parts_size = part_count * sizeof(uint32_t);
+ uint32_t size = info_size + field_size + colls_size + parts_size;
+
+ struct sql_space_info *info = sqlDbMallocRawNN(sql_get(), size);
+ if (info == NULL) {
+ diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
+ return NULL;
}
+ info->types = (enum field_type *)((char *)info + info_size);
+ info->coll_ids = (uint32_t *)((char *)info->types + field_size);
+ info->parts = part_count == 0 ? NULL :
+ (uint32_t *)((char *)info->coll_ids + colls_size);
+ info->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;
+}
+
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def)
+{
+ uint32_t field_count = def->field_count + 1;
+ struct sql_space_info *info = sql_space_info_new(field_count, 0);
+ if (info == NULL)
+ return NULL;
+ for (uint32_t i = 0; i < def->field_count; ++i) {
+ info->types[i] = def->fields[i].type;
+ info->coll_ids[i] = def->fields[i].coll_id;
+ }
+ /* Add one more field for rowid. */
+ info->types[def->field_count] = FIELD_TYPE_INTEGER;
+ return info;
+}
+
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid)
+{
+ uint32_t field_count = def->key_def->part_count;
+ if (has_rowid)
+ ++field_count;
+ struct sql_space_info *info = sql_space_info_new(field_count, 0);
+ if (info == NULL)
+ return NULL;
+ for (uint32_t i = 0; i < def->key_def->part_count; ++i) {
+ info->types[i] = def->key_def->parts[i].type;
+ info->coll_ids[i] = def->key_def->parts[i].coll_id;
+ }
+ if (has_rowid)
+ info->types[def->key_def->part_count] = FIELD_TYPE_INTEGER;
+ return info;
+}
+
+struct space *
+sql_ephemeral_space_new_from_info(const struct sql_space_info *info)
+{
+ uint32_t field_count = info->field_count;
+ uint32_t part_count = info->parts == NULL ? field_count :
+ info->part_count;
+ uint32_t parts_indent = field_count * sizeof(struct field_def);
+ uint32_t names_indent = part_count * sizeof(struct key_part_def) +
+ parts_indent;
/*
- * Name of the fields will be "_COLUMN_1", "_COLUMN_2"
- * and so on. Due to this, length of each name is no more
- * than strlen("_COLUMN_") plus length of UINT32_MAX
- * turned to string, which is 10 and plus 1 for \0.
+ * Name of the fields will be "_COLUMN_1", "_COLUMN_2" and so on. Due to
+ * this, length of each name is no more than 19 == strlen("_COLUMN_")
+ * plus length of UINT32_MAX turned to string, which is 10 and plus 1
+ * for '\0'.
*/
- uint32_t name_len = strlen("_COLUMN_") + 11;
- uint32_t size = field_count * (sizeof(struct field_def) + name_len) +
- part_count * sizeof(struct key_part_def);
+ uint32_t size = names_indent + field_count * 19;
+
+ struct region *region = &fiber()->gc;
+ size_t svp = region_used(region);
struct field_def *fields = region_aligned_alloc(region, size,
alignof(fields[0]));
if (fields == NULL) {
diag_set(OutOfMemory, size, "region_aligned_alloc", "fields");
return NULL;
}
- struct key_part_def *ephemer_key_parts =
- (void *)fields + field_count * sizeof(struct field_def);
- static_assert(alignof(*fields) == alignof(*ephemer_key_parts),
- "allocated in one block, and should have the same "
- "alignment");
- char *names = (char *)ephemer_key_parts +
- part_count * sizeof(struct key_part_def);
- for (uint32_t i = 0; i < field_count; ++i) {
- struct field_def *field = &fields[i];
- field->name = names;
- names += name_len;
- sprintf(field->name, "_COLUMN_%d", i);
- field->is_nullable = true;
- field->nullable_action = ON_CONFLICT_ACTION_NONE;
- field->default_value = NULL;
- field->default_value_expr = NULL;
- if (def != NULL && i < def->part_count) {
- assert(def->parts[i].type < field_type_MAX);
- field->type = def->parts[i].type;
- field->coll_id = def->parts[i].coll_id;
- } else {
- field->coll_id = COLL_NONE;
- field->type = FIELD_TYPE_SCALAR;
- }
+ struct key_part_def *parts = (struct key_part_def *)((char *)fields +
+ parts_indent);
+ static_assert(alignof(*fields) == alignof(*parts), "allocated in one "
+ "block, and should have the same alignment");
+ char *names = (char *)fields + names_indent;
+
+ for (uint32_t i = 0; i < info->field_count; ++i) {
+ fields[i].name = names;
+ sprintf(names, "_COLUMN_%d", i);
+ names += strlen(fields[i].name) + 1;
+ fields[i].is_nullable = true;
+ fields[i].nullable_action = ON_CONFLICT_ACTION_NONE;
+ fields[i].default_value = NULL;
+ fields[i].default_value_expr = NULL;
+ fields[i].type = info->types[i];
+ fields[i].coll_id = info->coll_ids[i];
}
-
for (uint32_t i = 0; i < part_count; ++i) {
- struct key_part_def *part = &ephemer_key_parts[i];
- /*
- * In case we need to save initial order of
- * inserted into ephemeral space rows we use rowid
- * as the only part of PK. If ephemeral space has
- * a rowid, it is always the last column.
- */
- uint32_t j = i;
- if (key_info != NULL && key_info->is_pk_rowid)
- j = field_count - 1;
- part->fieldno = j;
- part->nullable_action = ON_CONFLICT_ACTION_NONE;
- part->is_nullable = true;
- part->exclude_null = false;
- part->sort_order = SORT_ORDER_ASC;
- part->path = NULL;
- part->type = fields[j].type;
- part->coll_id = fields[j].coll_id;
+ uint32_t j = info->parts == NULL ? i : info->parts[i];
+ parts[i].fieldno = j;
+ parts[i].nullable_action = ON_CONFLICT_ACTION_NONE;
+ parts[i].is_nullable = true;
+ parts[i].exclude_null = false;
+ parts[i].sort_order = info->sort_order;
+ parts[i].path = NULL;
+ parts[i].type = info->types[j];
+ parts[i].coll_id = info->coll_ids[j];
}
- struct key_def *ephemer_key_def = key_def_new(ephemer_key_parts,
- part_count, false);
- if (ephemer_key_def == NULL)
+
+ struct key_def *key_def = key_def_new(parts, part_count, false);
+ if (key_def == NULL)
return NULL;
- struct index_def *ephemer_index_def =
- index_def_new(0, 0, "ephemer_idx", strlen("ephemer_idx"), TREE,
- &index_opts_default, ephemer_key_def, NULL);
- key_def_delete(ephemer_key_def);
- if (ephemer_index_def == NULL)
+ const char *name = "ephemer_idx";
+ struct index_def *index_def = index_def_new(0, 0, name, strlen(name),
+ TREE, &index_opts_default,
+ key_def, NULL);
+ key_def_delete(key_def);
+ if (index_def == NULL)
return NULL;
struct rlist key_list;
rlist_create(&key_list);
- rlist_add_entry(&key_list, ephemer_index_def, link);
+ rlist_add_entry(&key_list, index_def, link);
- struct space_def *ephemer_space_def =
- space_def_new_ephemeral(field_count, fields);
- if (ephemer_space_def == NULL) {
- index_def_delete(ephemer_index_def);
+ struct space_def *space_def = space_def_new_ephemeral(field_count,
+ fields);
+ if (space_def == NULL) {
+ index_def_delete(index_def);
return NULL;
}
- struct space *ephemer_new_space = space_new_ephemeral(ephemer_space_def,
- &key_list);
- index_def_delete(ephemer_index_def);
- space_def_delete(ephemer_space_def);
+ struct space *space = space_new_ephemeral(space_def, &key_list);
+ index_def_delete(index_def);
+ space_def_delete(space_def);
+ region_truncate(region, svp);
- return ephemer_new_space;
+ return space;
}
int tarantoolsqlEphemeralInsert(struct space *space, const char *tuple,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5226dd6ea..b82a6874e 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -224,12 +224,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* is held in ephemeral table, there is no PK for
* it, so columns should be loaded manually.
*/
- struct sql_key_info *pk_info = NULL;
int reg_eph = ++parse->nMem;
int reg_pk = parse->nMem + 1;
- int pk_len;
+ int pk_len = is_view ? space->def->field_count + 1 :
+ space->index[0]->def->key_def->part_count;
int eph_cursor = parse->nTab++;
int addr_eph_open = sqlVdbeCurrentAddr(v);
+ struct sql_space_info *info;
if (is_view) {
/*
* At this stage SELECT is already materialized
@@ -249,22 +250,20 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* account that id field as well. That's why pk_len
* has one field more than view format.
*/
- pk_len = space->def->field_count + 1;
- parse->nMem += pk_len;
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
- pk_len);
+ info = sql_space_info_new_from_space_def(space->def);
} else {
assert(space->index_count > 0);
- pk_info = sql_key_info_new_from_key_def(db,
- space->index[0]->def->key_def);
- if (pk_info == NULL)
- goto delete_from_cleanup;
- pk_len = pk_info->part_count;
- parse->nMem += pk_len;
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph,
- pk_len, 0,
- (char *)pk_info, P4_KEYINFO);
+ struct index_def *index_def = space->index[0]->def;
+ info = sql_space_info_new_from_index_def(index_def,
+ false);
}
+ if (info == NULL) {
+ parse->is_aborted = true;
+ goto delete_from_cleanup;
+ }
+ parse->nMem += pk_len;
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+ P4_DYNAMIC);
/* Construct a query to find the primary key for
* every row to be deleted, based on the WHERE
@@ -295,8 +294,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
/* Extract the primary key for the current row */
if (!is_view) {
- struct key_part_def *part = pk_info->parts;
- for (int i = 0; i < pk_len; i++, part++) {
+ struct key_def *def = space->index[0]->def->key_def;
+ for (int i = 0; i < pk_len; i++) {
+ struct key_part *part = &def->parts[i];
sqlVdbeAddOp3(v, OP_Column, tab_cursor,
part->fieldno, reg_pk + i);
}
@@ -373,7 +373,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (one_pass != ONEPASS_OFF) {
/* OP_Found will use an unpacked key. */
assert(key_len == pk_len);
- assert(pk_info != NULL || space->def->opts.is_view);
sqlVdbeAddOp4Int(v, OP_NotFound, tab_cursor,
addr_bypass, reg_key, key_len);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 6f40183ac..8902c648f 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2740,7 +2740,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
switch (pExpr->op) {
case TK_IN:{
- int addr; /* Address of OP_OpenEphemeral instruction */
Expr *pLeft = pExpr->pLeft; /* the LHS of the IN operator */
int nVal; /* Size of vector pLeft */
@@ -2761,13 +2760,14 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
*/
pExpr->iTable = pParse->nTab++;
int reg_eph = ++pParse->nMem;
- addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral,
- reg_eph, nVal);
+ struct sql_space_info *info =
+ sql_space_info_new(nVal, 0);
+ if (info == NULL)
+ return 0;
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, pExpr->iTable, 0,
reg_eph);
- struct sql_key_info *key_info = sql_key_info_new(pParse->db, nVal);
- if (key_info == NULL)
- return 0;
if (ExprHasProperty(pExpr, EP_xIsSelect)) {
/* Case 1: expr IN (SELECT ...)
@@ -2797,7 +2797,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
(pParse, pSelect, &dest)) {
sqlDbFree(pParse->db,
dest.dest_type);
- sql_key_info_unref(key_info);
return 0;
}
sqlDbFree(pParse->db,
@@ -2809,7 +2808,7 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
sqlVectorFieldSubexpr
(pLeft, i);
if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr,
- &key_info->parts[i].coll_id) != 0)
+ &info->coll_ids[i]) != 0)
return 0;
}
}
@@ -2829,7 +2828,7 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
bool unused;
struct coll *unused_coll;
if (sql_expr_coll(pParse, pExpr->pLeft, &unused,
- &key_info->parts[0].coll_id,
+ &info->coll_ids[0],
&unused_coll) != 0)
return 0;
@@ -2861,8 +2860,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
sqlReleaseTempReg(pParse, r1);
sqlReleaseTempReg(pParse, r2);
}
- sqlVdbeChangeP4(v, addr, (void *)key_info,
- P4_KEYINFO);
break;
}
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 21b4f2407..cc7b6b0e3 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -60,6 +60,27 @@ sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg)
(char *)colls_type, P4_DYNAMIC);
}
+static struct sql_space_info *
+sql_space_info_from_id_list(const struct space_def *def, int len,
+ const struct IdList *list)
+{
+ struct sql_space_info *info = sql_space_info_new(len + 1, 1);
+ if (info == NULL)
+ return NULL;
+ assert(len > 0 && len <= (int)def->field_count);
+ struct field_def *fields = def->fields;
+ for (int i = 0; i < len; ++i) {
+ int j = list == NULL ? i : list->a[i].idx;
+ info->types[i] = fields[j].type;
+ info->coll_ids[i] = fields[j].coll_id;
+ }
+ /* Add one more field for rowid. */
+ info->types[len] = FIELD_TYPE_INTEGER;
+ /* Set rowid as the only part of primary index. */
+ info->parts[0] = len;
+ return info;
+}
+
/**
* In SQL table can be created with AUTOINCREMENT.
* In Tarantool it can be detected as primary key which consists
@@ -442,34 +463,29 @@ sqlInsert(Parse * pParse, /* Parser context */
reg_eph = ++pParse->nMem;
regRec = sqlGetTempReg(pParse);
regCopy = sqlGetTempRange(pParse, nColumn + 1);
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
- nColumn + 1);
/*
- * This key_info is used to show that
- * rowid should be the first part of PK in
- * case we used AUTOINCREMENT feature.
- * This way we will save initial order of
- * the inserted values. The order is
- * important if we use the AUTOINCREMENT
- * feature, since changing the order can
- * change the number inserted instead of
- * NULL.
+ * Order of inserted values is important since it is
+ * possible, that NULL will be inserted in field with
+ * AUTOINCREMENT. So, the first part of key should be
+ * rowid. Since each rowid is unique, we do not need any
+ * other parts.
*/
- if (space->sequence != NULL) {
- struct sql_key_info *key_info =
- sql_key_info_new(pParse->db,
- nColumn + 1);
- key_info->parts[nColumn].type =
- FIELD_TYPE_UNSIGNED;
- key_info->is_pk_rowid = true;
- sqlVdbeChangeP4(v, -1, (void *)key_info,
- P4_KEYINFO);
+ struct sql_space_info *info =
+ sql_space_info_from_id_list(space_def, nColumn,
+ pColumn);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto insert_cleanup;
}
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
addrL = sqlVdbeAddOp1(v, OP_Yield, dest.iSDParm);
VdbeCoverage(v);
sqlVdbeAddOp2(v, OP_NextIdEphemeral, reg_eph,
regCopy + nColumn);
sqlVdbeAddOp3(v, OP_Copy, regFromSelect, regCopy, nColumn-1);
+ sqlVdbeAddOp4(v, OP_ApplyType, regCopy, nColumn + 1, 0,
+ (char *)info->types, P4_STATIC);
sqlVdbeAddOp3(v, OP_MakeRecord, regCopy,
nColumn + 1, regRec);
/* Set flag to save memory allocating one by malloc. */
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index b9107fccc..4aed24822 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -99,6 +99,152 @@ struct SortCtx {
#define SORTFLAG_UseSorter 0x01 /* Use SorterOpen instead of OpenEphemeral */
#define SORTFLAG_DESC 0xF0
+static inline uint32_t
+multi_select_coll_seq(struct Parse *parser, struct Select *p, int n);
+
+static struct sql_space_info *
+sql_space_info_new_from_expr_list(struct Parse *parser, struct ExprList *list,
+ bool has_rowid)
+{
+ int n = list->nExpr;
+ if (has_rowid)
+ ++n;
+ struct sql_space_info *info = sql_space_info_new(n, 0);
+ if (info == NULL)
+ return NULL;
+ for (int i = 0; i < list->nExpr; ++i) {
+ bool b;
+ struct coll *coll;
+ struct Expr *expr = list->a[i].pExpr;
+ enum field_type type = sql_expr_type(expr);
+ /*
+ * Type ANY could mean that field was unresolved. We have no way
+ * but to set it as SCALAR, however this could lead to
+ * unexpected change of type.
+ */
+ if (type == FIELD_TYPE_ANY)
+ type = FIELD_TYPE_SCALAR;
+ uint32_t coll_id;
+ if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
+ return NULL;
+ info->types[i] = type;
+ info->coll_ids[i] = coll_id;
+ }
+ info->sort_order = list->a[0].sort_order;
+ if (has_rowid)
+ info->types[list->nExpr] = FIELD_TYPE_INTEGER;
+ return info;
+}
+
+static struct sql_space_info *
+sql_space_info_new_from_order_by(struct Parse *parser, struct Select *select,
+ struct ExprList *order_by)
+{
+ int n = order_by->nExpr + 2;
+ struct sql_space_info *info = sql_space_info_new(n, 0);
+ if (info == NULL)
+ return NULL;
+ for (int i = 0; i < order_by->nExpr; ++i) {
+ struct Expr *expr = order_by->a[i].pExpr;
+ enum field_type type = sql_expr_type(expr);
+ /*
+ * Type ANY could mean that field was unresolved. We have no way
+ * but to set it as SCALAR, however this could lead to
+ * unexpected change of type.
+ */
+ if (type == FIELD_TYPE_ANY)
+ type = FIELD_TYPE_SCALAR;
+ info->types[i] = type;
+ uint32_t *id = &info->coll_ids[i];
+ if ((expr->flags & EP_Collate) != 0) {
+ bool b;
+ struct coll *coll;
+ if (sql_expr_coll(parser, expr, &b, id, &coll) != 0)
+ return NULL;
+ continue;
+ }
+ uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1;
+ info->coll_ids[i] = multi_select_coll_seq(parser, select,
+ fieldno);
+ if (info->coll_ids[i] != COLL_NONE) {
+ const char *name = coll_by_id(info->coll_ids[i])->name;
+ order_by->a[i].pExpr =
+ sqlExprAddCollateString(parser, expr, name);
+ }
+ }
+ info->sort_order = order_by->a[0].sort_order;
+ info->types[order_by->nExpr] = FIELD_TYPE_INTEGER;
+ info->types[order_by->nExpr + 1] = FIELD_TYPE_VARBINARY;
+ return info;
+}
+
+static struct sql_space_info *
+sql_space_info_new_for_sorting(struct Parse *parser, struct ExprList *order_by,
+ struct ExprList *list, int start, bool has_rowid)
+{
+ /*
+ * Index consist of fields that were not included into index plus rowid,
+ * is has_rowid is TRUE.
+ */
+ uint32_t part_count = order_by->nExpr - start;
+ if (has_rowid)
+ ++part_count;
+ /*
+ * Number of fields is number of parts in index plus number of fields
+ * that is not appear in ORDER BY, but were in SELECT.
+ */
+ uint32_t field_count = part_count;
+ /* If iOrderByCol != 0 than fields appear in ORDER BY. */
+ for (int i = 0; i < list->nExpr; ++i)
+ field_count += list->a[i].u.x.iOrderByCol == 0 ? 1 : 0;
+
+ struct sql_space_info *info =
+ sql_space_info_new(field_count, part_count);
+ if (info == NULL)
+ return NULL;
+ int k;
+ for (k = 0; k < order_by->nExpr - start; ++k) {
+ bool b;
+ struct coll *coll;
+ struct Expr *expr = order_by->a[k + start].pExpr;
+ enum field_type type = sql_expr_type(expr);
+ /*
+ * Type ANY could mean that field was unresolved. We have no way
+ * but to set it as SCALAR, however this could lead to
+ * unexpected change of type.
+ */
+ if (type == FIELD_TYPE_ANY)
+ type = FIELD_TYPE_SCALAR;
+ uint32_t coll_id;
+ if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0)
+ return NULL;
+ info->types[k] = type;
+ info->coll_ids[k] = coll_id;
+ }
+ if (has_rowid)
+ info->types[k++] = FIELD_TYPE_INTEGER;
+ assert(k == (int)part_count);
+ for (int i = 0; i < list->nExpr; ++i) {
+ if (list->a[i].u.x.iOrderByCol != 0)
+ continue;
+ bool b;
+ struct Expr *expr = list->a[i].pExpr;
+ enum field_type type = sql_expr_type(expr);
+ if (type == FIELD_TYPE_ANY)
+ type = FIELD_TYPE_SCALAR;
+ uint32_t id;
+ struct coll *coll;
+ if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0)
+ return NULL;
+ info->types[k] = type;
+ info->coll_ids[k] = id;
+ ++k;
+ }
+ assert(k == (int)field_count);
+ info->sort_order = order_by->a[0].sort_order;
+ return info;
+}
+
/*
* Delete all the content of a Select structure. Deallocate the structure
* itself only if bFree is true.
@@ -823,9 +969,26 @@ 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 sql_space_info *info =
+ sql_space_info_new_for_sorting(pParse,
+ pSort->pOrderBy,
+ pSelect->pEList,
+ nOBSat, bSeq);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ return;
+ }
+ pOp->p4.space_info = info;
+ pOp->p4type = P4_DYNAMIC;
+ }
addrJmp = sqlVdbeCurrentAddr(v);
sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1);
VdbeCoverage(v);
@@ -1044,13 +1207,28 @@ selectInnerLoop(Parse * pParse, /* The parser context */
* space format.
*/
uint32_t excess_field_count = 0;
+ struct VdbeOp *op = sqlVdbeGetOp(v,
+ pSort->addrSortIndex);
+ struct sql_space_info *info;
+ if (op->p4type == P4_KEYINFO)
+ info = op->p4.key_info->info;
+ else
+ info = op->p4.space_info;
for (i = pSort->nOBSat; i < pSort->pOrderBy->nExpr;
i++) {
int j = pSort->pOrderBy->a[i].u.x.iOrderByCol;
- 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 +1594,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 +1623,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 +1649,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,22 +2634,26 @@ generateWithRecursiveQuery(Parse * pParse, /* Parsing context */
/* Allocate cursors for Current, Queue, and Distinct. */
regCurrent = ++pParse->nMem;
sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol);
+ struct sql_space_info *info;
if (pOrderBy) {
- struct sql_key_info *key_info =
- sql_multiselect_orderby_to_key_info(pParse, p, 1);
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue,
- pOrderBy->nExpr + 2, 0, (char *)key_info,
- P4_KEYINFO);
VdbeComment((v, "Orderby table"));
+ info = sql_space_info_new_from_order_by(pParse, p, pOrderBy);
destQueue.pOrderBy = pOrderBy;
} else {
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1);
VdbeComment((v, "Queue table"));
+ info = sql_space_info_new_from_expr_list(pParse, p->pEList,
+ true);
}
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto end_of_recursive_query;
+ }
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, 0, 0, (char *)info,
+ P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue);
if (iDistinct) {
p->addrOpenEphm[0] =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_dist, 1);
+ sqlVdbeAddOp1(v, OP_OpenTEphemeral, reg_dist);
sqlVdbeAddOp3(v, OP_IteratorOpen, iDistinct, 0, reg_dist);
p->selFlags |= SF_UsesEphemeral;
VdbeComment((v, "Distinct table"));
@@ -2672,8 +2857,16 @@ multiSelect(Parse * pParse, /* Parsing context */
*/
if (dest.eDest == SRT_EphemTab) {
assert(p->pEList);
- int nCols = p->pEList->nExpr;
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, p->pEList,
+ true);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ rc = 1;
+ goto multi_select_end;
+ }
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph);
VdbeComment((v, "Destination temp"));
dest.eDest = SRT_Table;
@@ -2789,10 +2982,9 @@ multiSelect(Parse * pParse, /* Parsing context */
unionTab = pParse->nTab++;
reg_union = ++pParse->nMem;
assert(p->pOrderBy == 0);
- addr =
- sqlVdbeAddOp2(v,
- OP_OpenTEphemeral,
- reg_union, 0);
+ addr = sqlVdbeAddOp1(v,
+ OP_OpenTEphemeral,
+ reg_union);
sqlVdbeAddOp3(v, OP_IteratorOpen, unionTab, 0, reg_union);
assert(p->addrOpenEphm[0] == -1);
p->addrOpenEphm[0] = addr;
@@ -2905,9 +3097,8 @@ multiSelect(Parse * pParse, /* Parsing context */
reg_eph2 = ++pParse->nMem;
assert(p->pOrderBy == 0);
- addr =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph1,
- 0);
+ addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+ reg_eph1);
sqlVdbeAddOp3(v, OP_IteratorOpen, tab1, 0, reg_eph1);
assert(p->addrOpenEphm[0] == -1);
p->addrOpenEphm[0] = addr;
@@ -2927,9 +3118,8 @@ multiSelect(Parse * pParse, /* Parsing context */
/* Code the current SELECT into temporary table "tab2"
*/
- addr =
- sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph2,
- 0);
+ addr = sqlVdbeAddOp1(v, OP_OpenTEphemeral,
+ reg_eph2);
sqlVdbeAddOp3(v, OP_IteratorOpen, tab2, 0, reg_eph2);
assert(p->addrOpenEphm[1] == -1);
p->addrOpenEphm[1] = addr;
@@ -3001,14 +3191,14 @@ multiSelect(Parse * pParse, /* Parsing context */
if (p->selFlags & SF_UsesEphemeral) {
assert(p->pNext == NULL);
int nCol = p->pEList->nExpr;
- struct sql_key_info *key_info = sql_key_info_new(db, nCol);
- if (key_info == NULL)
+ struct sql_space_info *info = sql_space_info_new(nCol, 0);
+ if (info == NULL) {
+ pParse->is_aborted = true;
goto multi_select_end;
- for (int i = 0; i < nCol; i++) {
- key_info->parts[i].coll_id =
- multi_select_coll_seq(pParse, p, i);
}
-
+ for (int i = 0; i < nCol; ++i)
+ info->coll_ids[i] = multi_select_coll_seq(pParse, p, i);
+ bool is_info_used = false;
for (struct Select *pLoop = p; pLoop; pLoop = pLoop->pPrior) {
for (int i = 0; i < 2; i++) {
int addr = pLoop->addrOpenEphm[i];
@@ -3020,13 +3210,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 +5539,22 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo)
"exactly one argument");
pParse->is_aborted = true;
pFunc->iDistinct = -1;
- } else {
- struct sql_key_info *key_info =
- sql_expr_list_to_key_info(pParse,
+ return;
+ }
+ assert(pE->x.pList->nExpr == 1);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse,
pE->x.pList,
- 0);
- sqlVdbeAddOp4(v, OP_OpenTEphemeral,
- pFunc->reg_eph, 1, 0,
- (char *)key_info, P4_KEYINFO);
- sqlVdbeAddOp3(v, OP_IteratorOpen,
- pFunc->iDistinct, 0, pFunc->reg_eph);
+ false);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ pFunc->iDistinct = -1;
+ return;
}
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 0,
+ 0, (char *)info, P4_DYNAMIC);
+ sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0,
+ pFunc->reg_eph);
}
}
}
@@ -5874,11 +6071,26 @@ sqlSelect(Parse * pParse, /* The parser context */
if (key_info->parts[0].sort_order == SORT_ORDER_DESC) {
sSort.sortFlags |= SORTFLAG_DESC;
}
+ /*
+ * We need to defined both sql_key_info and sql_space_info since
+ * this opcode may change to OP_SorterOpen, which uses
+ * sql_key_info. This is also the reason why we should give p2
+ * for this opcode.
+ */
sSort.addrSortIndex =
sqlVdbeAddOp4(v, OP_OpenTEphemeral,
sSort.reg_eph,
nCols,
0, (char *)key_info, P4_KEYINFO);
+ struct sql_space_info *info =
+ sql_space_info_new_for_sorting(pParse, sSort.pOrderBy,
+ pEList, 0, true);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto select_end;
+ }
+ key_info->info = info;
+
sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph);
VdbeComment((v, "Sort table"));
} else {
@@ -5888,11 +6100,14 @@ sqlSelect(Parse * pParse, /* The parser context */
/* If the output is destined for a temporary table, open that table.
*/
if (pDest->eDest == SRT_EphemTab) {
- struct sql_key_info *key_info =
- sql_expr_list_to_key_info(pParse, pEList, 0);
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph,
- pEList->nExpr + 1, 0, (char *)key_info,
- P4_KEYINFO);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, pEList, true);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto select_end;
+ }
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
pDest->reg_eph);
@@ -5918,13 +6133,16 @@ sqlSelect(Parse * pParse, /* The parser context */
if (p->selFlags & SF_Distinct) {
sDistinct.cur_eph = pParse->nTab++;
sDistinct.reg_eph = ++pParse->nMem;
- struct sql_key_info *key_info =
- sql_expr_list_to_key_info(pParse, p->pEList, 0);
+ struct sql_space_info *info =
+ sql_space_info_new_from_expr_list(pParse, pEList,
+ false);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto select_end;
+ }
sDistinct.addrTnct = sqlVdbeAddOp4(v, OP_OpenTEphemeral,
- sDistinct.reg_eph,
- key_info->part_count,
- 0, (char *)key_info,
- P4_KEYINFO);
+ sDistinct.reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, sDistinct.cur_eph, 0,
sDistinct.reg_eph);
VdbeComment((v, "Distinct table"));
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1f87e6823..2564eda8a 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 sql_space_info *info;
/**
* Key definition created from this object,
* see sql_key_info_to_key_def().
@@ -4055,6 +4057,64 @@ sql_key_info_unref(struct sql_key_info *key_info);
struct key_def *
sql_key_info_to_key_def(struct sql_key_info *key_info);
+/**
+ * Structure that is used to store information about ephemeral space field types
+ * and fieldno of key parts.
+ */
+struct sql_space_info {
+ /** Field types of all fields of ephemeral space. */
+ enum field_type *types;
+ /** Collation ids of all fields of ephemeral space. */
+ uint32_t *coll_ids;
+ /**
+ * Fieldno key parts of the ephemeral space. If NULL, then the index
+ * consists of all fields in sequential order.
+ */
+ uint32_t *parts;
+ /** 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 sql_space_info *
+sql_space_info_new(uint32_t field_count, uint32_t part_count);
+
+/**
+ * Initialize the field types and key parts of space_info with space_def.
+ * Additionally added one more field type and key part for rowid. Rowid is
+ * always INTEGER. Key parts will be initialized with the same values as the
+ * field types. The number of initialized field types and key parts will be the
+ * same as the field_count in space_def plus one.
+ */
+struct sql_space_info *
+sql_space_info_new_from_space_def(const struct space_def *def);
+
+/**
+ * Initialize the field types and key parts of space_info with index_def.
+ * Key parts will be initialized with the same values as the field types. The
+ * number of initialized field types and key parts will be the same as the
+ * part_count in index_def.
+ */
+struct sql_space_info *
+sql_space_info_new_from_index_def(const struct index_def *def, bool has_rowid);
+
+/**
+ * Allocate and initialize an ephemeral space. Information about field types and
+ * key parts is taken from the space_info structure.
+ */
+struct space *
+sql_ephemeral_space_new_from_info(const struct sql_space_info *info);
+
/**
* Check if the function implements LIKE-style comparison & if it
* is appropriate to apply a LIKE query optimization.
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 8fdc50432..adb7b5f9f 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -61,21 +61,6 @@ sql_rename_table(uint32_t space_id, const char *new_name);
int tarantoolsqlRenameTrigger(const char *zTriggerName,
const char *zOldName, const char *zNewName);
-/**
- * Create ephemeral space. Features of ephemeral spaces: id == 0,
- * name == "ephemeral", memtx engine (in future it can be changed,
- * but now only memtx engine is supported), primary index which
- * covers all fields and no secondary indexes, given field number
- * and collation sequence. All fields are scalar and nullable.
- *
- * @param field_count Number of fields in ephemeral space.
- * @param key_info Keys description for new ephemeral space.
- *
- * @retval Pointer to created space, NULL if error.
- */
-struct space *
-sql_ephemeral_space_create(uint32_t filed_count, struct sql_key_info *key_info);
-
/**
* Insert tuple into ephemeral space.
* In contrast to ordinary spaces, there is no need to create and
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 22f82390c..b4827f242 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -226,9 +226,20 @@ sqlUpdate(Parse * pParse, /* The parser context */
iEph = pParse->nTab++;
sqlVdbeAddOp2(v, OP_Null, 0, iPk);
+ struct sql_space_info *info;
+ assert(space->index_count > 0 || is_view);
+ if (is_view)
+ info = sql_space_info_new_from_space_def(def);
+ else
+ info = sql_space_info_new_from_index_def(pPk->def, false);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ goto update_cleanup;
+ }
/* Address of the OpenEphemeral instruction. */
- int addrOpen = sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph,
- pk_part_count);
+ int addrOpen = sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0,
+ (char *)info, P4_DYNAMIC);
+
pWInfo = sqlWhereBegin(pParse, pTabList, pWhere, 0, 0,
WHERE_ONEPASS_DESIRED, pk_cursor);
if (pWInfo == 0)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index fcea9eefe..0facd75c7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2033,10 +2033,9 @@ case OP_Fetch: {
case OP_ApplyType: {
enum field_type *types = pOp->p4.types;
assert(types != NULL);
- assert(types[pOp->p2] == field_type_MAX);
pIn1 = &aMem[pOp->p1];
- enum field_type type;
- while((type = *(types++)) != field_type_MAX) {
+ for (int i = 0; i < pOp->p2; ++i, ++pIn1) {
+ enum field_type type = types[i];
assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]);
assert(memIsValid(pIn1));
if (mem_cast_implicit(pIn1, type) != 0) {
@@ -2044,7 +2043,6 @@ case OP_ApplyType: {
mem_str(pIn1), field_type_strs[type]);
goto abort_due_to_error;
}
- pIn1++;
}
break;
}
@@ -2380,10 +2378,9 @@ open_cursor_set_hints:
}
/**
- * Opcode: OpenTEphemeral P1 P2 * P4 *
+ * Opcode: OpenTEphemeral P1 * * P4 *
* Synopsis:
* @param P1 register, where pointer to new space is stored.
- * @param P2 number of columns in a new table.
* @param P4 key def for new table, NULL is allowed.
*
* This opcode creates Tarantool's ephemeral table and stores pointer
@@ -2391,11 +2388,12 @@ 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 sql_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..e40a1a0b3 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -93,6 +93,10 @@ struct VdbeOp {
* doing a cast.
*/
enum field_type *types;
+ /**
+ * Information about ephemeral space field types and key parts.
+ */
+ struct sql_space_info *space_info;
} p4;
#ifdef SQL_ENABLE_EXPLAIN_COMMENTS
char *zComment; /* Comment to improve readability */
@@ -210,15 +214,6 @@ int sqlVdbeDeletePriorOpcode(Vdbe *, u8 op);
void sqlVdbeChangeP4(Vdbe *, int addr, const char *zP4, int N);
void sqlVdbeAppendP4(Vdbe *, void *pP4, int p4type);
-/**
- * Set the P4 on the most recently added opcode to the key_def for the
- * index given.
- * @param Parse context, for error reporting.
- * @param key_def Definition of a key to set.
- */
-void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def);
-
VdbeOp *sqlVdbeGetOp(Vdbe *, int);
int sqlVdbeMakeLabel(Vdbe *);
void sqlVdbeRunOnlyOnce(Vdbe *);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 61be7b489..2d7800b17 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -787,18 +787,6 @@ sqlVdbeAppendP4(Vdbe * p, void *pP4, int n)
}
}
-void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def)
-{
- struct Vdbe *v = parse->pVdbe;
- assert(v != NULL);
- assert(key_def != NULL);
- struct sql_key_info *key_info =
- sql_key_info_new_from_key_def(parse->db, key_def);
- if (key_info != NULL)
- sqlVdbeAppendP4(v, key_info, P4_KEYINFO);
-}
-
#ifdef SQL_ENABLE_EXPLAIN_COMMENTS
/*
* Change the comment on the most recently coded instruction. Or
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 16766f2f8..d8d23161b 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -919,15 +919,15 @@ constructAutomaticIndex(Parse * pParse, /* The parsing context */
/* Create the automatic index */
assert(pLevel->iIdxCur >= 0);
pLevel->iIdxCur = pParse->nTab++;
- struct sql_key_info *pk_info =
- sql_key_info_new_from_key_def(pParse->db, idx_def->key_def);
- if (pk_info == NULL) {
+ struct sql_space_info *info = sql_space_info_new_from_index_def(idx_def,
+ true);
+ if (info == NULL) {
pParse->is_aborted = true;
return;
}
int reg_eph = sqlGetTempReg(pParse);
- sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nKeyCol + 1, 0,
- (char *)pk_info, P4_KEYINFO);
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, 0, 0, (char *)info,
+ P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, pLevel->iIdxCur, 0, reg_eph);
VdbeComment((v, "for %s", space->def->name));
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index df6cc92e1..0d0cb054d 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1134,11 +1134,19 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W
if ((pWInfo->wctrlFlags & WHERE_DUPLICATES_OK) == 0) {
cur_row_set = pParse->nTab++;
reg_row_set = ++pParse->nMem;
- sqlVdbeAddOp2(v, OP_OpenTEphemeral,
- reg_row_set, pk_part_count);
+ struct index_def *index_def = space->index[0]->def;
+ struct sql_space_info *info =
+ sql_space_info_new_from_index_def(index_def,
+ false);
+ if (info == NULL) {
+ pParse->is_aborted = true;
+ return notReady;
+ }
+ sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set,
+ pk_part_count, 0, (char *)info,
+ P4_DYNAMIC);
sqlVdbeAddOp3(v, OP_IteratorOpen, cur_row_set, 0,
reg_row_set);
- sql_vdbe_set_p4_key_def(pParse, pk_key_def);
regPk = ++pParse->nMem;
}
iRetInit = sqlVdbeAddOp2(v, OP_Integer, 0, regReturn);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields
2021-08-15 13:26 ` Mergen Imeev via Tarantool-patches
@ 2021-08-16 8:57 ` Vladimir Davydov via Tarantool-patches
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Davydov via Tarantool-patches @ 2021-08-16 8:57 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches, Mergen Imeev
On Sun, Aug 15, 2021 at 04:26:03PM +0300, Mergen Imeev wrote:
> On Fri, Aug 13, 2021 at 06:12:29PM +0300, Vladimir Davydov wrote:
> > On Fri, Aug 13, 2021 at 06:09:19AM +0300, imeevma@tarantool.org wrote:
> > > diff --git a/src/box/sql.c b/src/box/sql.c
> > > index c805a1e5c..2b95aed8d 100644
> > > --- a/src/box/sql.c
> > > +++ b/src/box/sql.c
> > > @@ -243,114 +243,163 @@ tarantoolsqlCount(struct BtCursor *pCur)
> > > return index_count(pCur->index, pCur->iter_type, NULL, 0);
> > > }
> > >
> > > -struct space *
> > > -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
> > > +struct space_info *
> > > +sql_space_info_new(uint32_t field_count, uint32_t part_count)
> > > {
> > > - struct key_def *def = NULL;
> > > - uint32_t part_count = field_count;
> > > - if (key_info != NULL) {
> > > - def = sql_key_info_to_key_def(key_info);
> > > - if (def == NULL)
> > > - return NULL;
> > > - /*
> > > - * In case is_pk_rowid is true we can use rowid
> > > - * as the only part of the key.
> > > - */
> > > - if (key_info->is_pk_rowid)
> > > - part_count = 1;
> > > + assert(field_count > 0);
> > > + uint32_t info_size = sizeof(struct space_info);
> > > + uint32_t field_size = field_count * sizeof(enum field_type);
> > > + uint32_t colls_size = field_count * sizeof(uint32_t);
> > > + uint32_t parts_size = part_count * sizeof(uint32_t);
> > > + uint32_t size = info_size + field_size + colls_size + parts_size;
> > > +
> > > + struct space_info *info = sqlDbMallocRawNN(sql_get(), size);
> > > + if (info == NULL) {
> > > + diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info");
> > > + return NULL;
> > > }
> > > + info->types = (enum field_type *)((char *)info + info_size);
> > > + info->coll_ids = (uint32_t *)((char *)info->types + field_size);
> > > + info->parts = part_count == 0 ? NULL :
> > > + (uint32_t *)((char *)info->coll_ids + colls_size);
> > > + info->field_count = field_count;
> > > + info->part_count = part_count;
> > > + info->sort_order = SORT_ORDER_ASC;
> > >
> > > - struct region *region = &fiber()->gc;
> > > + for (uint32_t i = 0; i < field_count; ++i) {
> > > + info->types[i] = FIELD_TYPE_SCALAR;
> > > + info->coll_ids[i] = COLL_NONE;
> > > + }
> > > + for (uint32_t i = 0; i < part_count; ++i)
> > > + info->parts[i] = i;
> > > + return info;
> > > +}
> > > +
> > > +void
> > > +sql_space_info_from_space_def(struct space_info *info,
> > > + const struct space_def *def)
> >
> > Separating allocation from initialization only complicates the protocol:
> > - The caller must ensure that the arguments passed to alloc and init
> > are compatible.
> > - There's a window between alloc and init when the object is unusable.
> >
> > Please don't:
> >
> > struct sql_space_info *
> > sql_space_info_new_from_space_def(...);
> >
> > struct sql_space_info *
> > sql_space_info_new_from_index_def(...);
> >
> Fixed. There is still some places that cannot be fixed for now, since
> sql_space_info may be modified during execution of some code.
Please also rename sql_space_info_from_id_list to
sql_space_info_new_from_id_list.
> > struct space *
> > sql_ephemeral_space_new(const struct sql_space_info *info);
> >
> I would like to do this, but we already have a function with such name. More
> than that, this functions actually have no connection to ephemeral spaces,
> the "spaces" it creates are only needed for creation of normal spaces, i.e.
> it actually creates analogues of space_def. Not sure why space_def cannot be
> used there.
Then you should give those old functions adequate names.
Please rename them in a separate patch and rebase.
> > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> > > index b9107fccc..a3e551017 100644
> > > --- a/src/box/sql/select.c
> > > +++ b/src/box/sql/select.c
> > > @@ -5879,6 +6047,23 @@ sqlSelect(Parse * pParse, /* The parser context */
> > > sSort.reg_eph,
> > > nCols,
> > > 0, (char *)key_info, P4_KEYINFO);
> > > + /*
> > > + * We also should define space_info in case this opcode will not
> > > + * be converted to SorterOpen.
> > > + */
> > > + uint32_t part_count = sSort.pOrderBy->nExpr + 1;
> > > + struct space_info *info = sql_space_info_new(nCols, part_count);
> > > + if (info == NULL) {
> > > + pParse->is_aborted = true;
> > > + goto select_end;
> > > + }
> > > + key_info->info = info;
> > > + if (space_info_for_sorting(info, pParse, sSort.pOrderBy,
> > > + pEList) != 0) {
> > > + pParse->is_aborted = true;
> > > + goto select_end;
> > > + }
> > > +
> >
> > I don't understand why you can't pass sql_space_info instead of
> > sql_key_info. AFAIU, you don't need key_info here at all anymore.
> > Anyway, keeping a pointer to space_info in key_info looks bad.
> > Can you avoid that?
> >
> The problem here is that OpenTEphemeral opcode can be replaced by SorterOpen
> opcode. Also, its key_info could be moved to OP_Compare opcode. Actually, I
> made a patch which also replaces sql_key_info by space_info for these two
> opcodes, but space_info become too complicated. I decided to leave it for now.
> I actually think that we should drop sorter and OP_Compare, but not sure that
> we should do it now, before release. I will think of it a bit later, if I will
> be given some time to rework this.
I think you should convert space_info to key_info when changing
the opcode then.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-16 8:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 3:09 [Tarantool-patches] [PATCH v1 1/1] sql: define ephemeral space fields Mergen Imeev via Tarantool-patches
2021-08-13 15:12 ` Vladimir Davydov via Tarantool-patches
2021-08-15 13:26 ` Mergen Imeev via Tarantool-patches
2021-08-16 8:57 ` Vladimir Davydov 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