From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 605886EC40; Tue, 10 Aug 2021 17:32:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 605886EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628605975; bh=1I8nx9kSwMs4TFAMPbDc7mDi+jBpn+ArHK5nZ/sjkSA=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=dpmXnj5Jg8SN9qoiKF+hRxrdbwNve3ciltUQS8XyV8fHvCC+HCxH3aTO2msvRAYr7 qqUg5jPfLSIEmDjZhK58ip9AwxuT/4pUJqTOGgicFgEeunPF4tfCkMWM7wqM2nQXU/ s49MAEe0vrrQ1J5+o1mUHfGITvYtQQDOAYRPrWgY= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 534EF6EC40 for ; Tue, 10 Aug 2021 17:32:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 534EF6EC40 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1mDSoG-0000sr-Ls; Tue, 10 Aug 2021 17:32:53 +0300 Date: Tue, 10 Aug 2021 17:32:51 +0300 To: Vladislav Shpilevoy Message-ID: <20210810143251.GA471410@tarantool.org> References: <30b6e5331a79e14a0fdd0e5782aa7ac29c4922bb.1626358375.git.imeevma@gmail.com> <41ee6127-c53c-c964-a71c-c4a264f59ab0@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <41ee6127-c53c-c964-a71c-c4a264f59ab0@tarantool.org> X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9D5AC6413C25DCF08CC98B8FCC5CD86F3182A05F53808504094E9224B337B7B2FDA0D738FB68AEB3024857C648FC220AD2B1E0B8A2B24BF6B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7260723E369DE2609EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E8DF562B6063DE0A8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8B848FB232833AC4709397A4141C31562117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B66F6A3E018CF4DC80089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5A36B499C76CF691E8CA1A0BC1E777DE9143522AD33D9A8DBD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753177526CD55AFC11410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34524B2D44961BF9C6EACDE2E72200E149042DB70C1E7E91B4C2D0BB8699D0A25CF20E37DF1E6B36391D7E09C32AA3244C059661A5FB80CA6B90923351A9048ED4B018FE5BB746DCD1FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj6qlzQV0oSZMUSBrZy09aUA== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D1E63C3B0153EB4550360CC54599B754F83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: define format of ephemeral spaces X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thank you for the review! My answers and new patch below. Also, new patch is a bit bigger than the old one since I could't think of proper reason do divide it into more than one patch. On Sat, Jul 24, 2021 at 01:06:30AM +0200, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > See 9 comments below. > > > diff --git a/src/box/sql.c b/src/box/sql.c > > index 790ca7f70..f5d0d6ce6 100644 > > --- a/src/box/sql.c > > +++ b/src/box/sql.c > > @@ -298,7 +298,8 @@ tarantoolsqlCount(struct BtCursor *pCur) > > } > > > > struct space * > > -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) > > +sql_ephemeral_space_create_key_info(uint32_t field_count, > > + struct sql_key_info *key_info) > > 1. The function creates not key_info. It creates a space by key_info. > Maybe call it sql_ephemeral_space_new_from_key_info()? > I dropped old function. A new function is now named sql_ephemeral_space_new_from_info(). > > { > > struct key_def *def = NULL; > > uint32_t part_count = field_count; > > @@ -407,6 +408,122 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) > > return ephemer_new_space; > > } > > > > +enum { > > + /* > > + * Name of the fields will be "_COLUMN_1", "_COLUMN_2" and so on. Due to > > + * this, length of each name is no more than strlen("_COLUMN_") plus > > + * length of UINT32_MAX turned to string, which is 10 and plus 1 for \0. > > + */ > > + NAME_LEN = 19, > > +}; > > + > > +static struct space_def * > > +sql_space_def_new_ephemeral(uint32_t field_count, enum field_type *types, > > + uint32_t *coll_ids) > > 2. The last 2 arguments can be made const. The same in the other functions > below. > Fixed for new function. > > +{ > > + struct region *region = &fiber()->gc; > > + size_t svp = region_used(region); > > + uint32_t size = field_count * sizeof(struct field_def); > > + struct field_def *fields = region_aligned_alloc(region, size, > > + alignof(fields[0])); > > 3. There is region_alloc_array(). The same in the next function. > I am not sure tha it is proper to use this function here since the memory alloced here will contain field_def, key_part_def and names of fields. > > + if (fields == NULL) { > > + diag_set(OutOfMemory, size, "region_aligned_alloc", "fields"); > > + return NULL; > > + } > > + char *names = region_alloc(region, field_count * NAME_LEN); > > + if (names == NULL) { > > + diag_set(OutOfMemory, size, "region_alloc", "names");> + return NULL; > > + } > > + for (uint32_t i = 0; i < field_count; ++i) { > > + struct field_def *field = &fields[i]; > > + field->name = &names[i * NAME_LEN]; > > + sprintf(field->name, "_COLUMN_%d", i); > > + field->is_nullable = true; > > + field->nullable_action = ON_CONFLICT_ACTION_NONE; > > + field->default_value = NULL; > > + field->default_value_expr = NULL; > > + field->type = types[i]; > > + field->coll_id = coll_ids[i]; > > + } > > + struct space_def *def = space_def_new_ephemeral(field_count, fields); > > + region_truncate(region, svp); > > + return def; > > +} > > + > > +static struct index_def * > > +sql_index_def_new_ephemeral(uint32_t field_count, enum field_type *types, > > + uint32_t *coll_ids, bool is_pk_rowid) > > +{ > > + uint32_t part_count = is_pk_rowid ? 1 : field_count; > > + struct region *region = &fiber()->gc; > > + size_t svp = region_used(region); > > + uint32_t size = part_count * sizeof(struct key_part_def); > > + struct key_part_def *parts = region_aligned_alloc(region, size, > > + alignof(parts[0])); > > + if (parts == NULL) { > > + diag_set(OutOfMemory, size, "region_aligned_alloc", "parts"); > > + return NULL; > > + } > > + if (is_pk_rowid) { > > + uint32_t j = field_count - 1; > > + parts[0].fieldno = j; > > + parts[0].nullable_action = ON_CONFLICT_ACTION_NONE; > > + parts[0].is_nullable = true; > > + parts[0].exclude_null = false; > > + parts[0].sort_order = SORT_ORDER_ASC; > > + parts[0].path = NULL; > > + parts[0].type = types[j]; > > + parts[0].coll_id = coll_ids[j]; > > + } else { > > + for (uint32_t i = 0; i < part_count; ++i) { > > + struct key_part_def *part = &parts[i]; > > + part->fieldno = i; > > + part->nullable_action = ON_CONFLICT_ACTION_NONE; > > + part->is_nullable = true; > > + part->exclude_null = false; > > + part->sort_order = SORT_ORDER_ASC; > > + part->path = NULL; > > + part->type = types[i]; > > + part->coll_id = coll_ids[i]; > > + } > > + } > > + struct key_def *key_def = key_def_new(parts, part_count, false); > > + if (key_def == NULL) > > + return NULL; > > + const char *name = "ephemer_idx"; > > + struct index_def *def = index_def_new(0, 0, name, strlen(name), TREE, > > + &index_opts_default, key_def, > > + NULL); > > + key_def_delete(key_def); > > + region_truncate(region, svp); > > + return def; > > +} > > + > > +struct space * > > +sql_ephemeral_space_create(uint32_t field_count, enum field_type *types, > > + uint32_t *coll_ids, bool is_pk_rowid) > > 4. It is strange how there are functions > > sql_space_def_new_ephemeral > sql_index_def_new_ephemeral > > and at the same time these: > > sql_ephemeral_space_create_key_info > sql_ephemeral_space_create > > First ones use ephemeral in the end, the last ones in the beginning. > I would propose to unify them while you are here. One of the > options: > > sql_ephemeral_space_def_new > sql_ephemeral_index_def_new > sql_ephemeral_space_new_from_key_info > sql_ephemeral_space_new > Thank you for suggestion, but I dropped most of these function. New functions: sql_ephemeral_space_new_from_info sql_space_info_from_space_def sql_space_info_from_index_def And couple of static functions: space_info_from_expr_list space_info_from_order_by > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > > index 62a726fdd..b35545296 100644 > > --- a/src/box/sql/delete.c > > +++ b/src/box/sql/delete.c > > @@ -249,22 +254,20 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > > * account that id field as well. That's why pk_len > > * has one field more than view format. > > */ > > - pk_len = space->def->field_count + 1; > > - parse->nMem += pk_len; > > - sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph, > > - pk_len); > > + uint32_t count = space->def->field_count; > > + fields_info_from_space_def(info->types, info->coll_ids, > > + count, space->def); > > 5. In all usages of fields_info_from_space_def you pass 2 fields of > field_info. I propose to make it take field_info pointer instead > of these 2 arrays. Also for individual fields I propose to add > field_info_set_field(type, coll_id). So as not to miss anything. Currently > it looks too much boilerplate code. > Fixed, I think. > > + info->types[count] = FIELD_TYPE_UNSIGNED; > > + info->coll_ids[count] = COLL_NONE; > > } else { > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > > index b9107fccc..c59d7b4b4 100644 > > --- a/src/box/sql/select.c > > +++ b/src/box/sql/select.c > > @@ -99,6 +99,103 @@ struct SortCtx { > > #define SORTFLAG_UseSorter 0x01 /* Use SorterOpen instead of OpenEphemeral */ > > #define SORTFLAG_DESC 0xF0 > > > > +static inline uint32_t > > +multi_select_coll_seq(struct Parse *parser, struct Select *p, int n); > > + > > +struct fields_info * > > +fields_info_new(uint32_t len) > > +{ > > + uint32_t size_info = sizeof(struct fields_info); > > + uint32_t size_types = len * sizeof(enum field_type); > > + uint32_t size = size_info + size_types + len * (sizeof(uint32_t)); > > + char *buf = sqlDbMallocRawNN(sql_get(), size); > > + struct fields_info *info = (struct fields_info *)buf; > > + if (info == NULL) > > + return NULL; > > + info->key_info = NULL; > > + buf += size_info; > > + info->types = (enum field_type *)buf; > > + buf += size_types; > > + info->coll_ids = (uint32_t *)buf; > > + return info; > > +} > > + > > +void > > +fields_info_delete(struct fields_info *info) > > +{ > > + if (info->key_info != NULL) > > + sql_key_info_unref(info->key_info); > > + return sqlDbFree(sql_get(), info); > > 6. You do not need this explicit 'return'. > Dropped this function. > > +} > > + > > +void > > +fields_info_from_space_def(enum field_type *types, uint32_t *coll_ids, > > + uint32_t count, struct space_def *def) > > 7. def can be made const. The same for some other arguments below. > Fixed. > > +{ > > + for (uint32_t i = 0; i < count; ++i) { > > + types[i] = def->fields[i].type; > > + coll_ids[i] = def->fields[i].coll_id; > > + } > > +} > > + > > +void > > +fields_info_from_index_def(enum field_type *types, uint32_t *coll_ids, > > + uint32_t count, struct index_def *def) > > +{ > > + for (uint32_t i = 0; i < count; ++i) { > > + types[i] = def->key_def->parts[i].type; > > + coll_ids[i] = def->key_def->parts[i].coll_id; > > + } > > +} > > + > > +static int > > +fields_info_from_expr_list(enum field_type *types, uint32_t *coll_ids, > > + struct Parse *parser, struct ExprList *list) > > +{ > > + for (int i = 0; i < list->nExpr; ++i) { > > + bool b; > > + struct coll *coll; > > + struct Expr *expr = list->a[i].pExpr; > > + types[i] = sql_expr_type(expr); > > + if (types[i] == FIELD_TYPE_ANY) > > + types[i] = FIELD_TYPE_SCALAR; > > + if (sql_expr_coll(parser, expr, &b, &coll_ids[i], &coll) != 0) > > + return -1; > > + } > > + return 0; > > +} > > + > > +static int > > +fields_info_from_order_by(enum field_type *types, uint32_t *coll_ids, > > + struct Parse *parser, struct Select *select, > > + struct ExprList *order_by) > > +{ > > + for (int i = 0; i < order_by->nExpr; ++i) { > > + bool b; > > + struct Expr *expr = order_by->a[i].pExpr; > > + types[i] = sql_expr_type(expr); > > + if (types[i] == FIELD_TYPE_ANY) > > + types[i] = FIELD_TYPE_SCALAR; > > + uint32_t id = COLL_NONE; > > + if ((expr->flags & EP_Collate) != 0) { > > + struct coll *coll; > > + if (sql_expr_coll(parser, expr, &b, &id, &coll) != 0) > > + return -1; > > + } else { > > + uint32_t fieldno = order_by->a[i].u.x.iOrderByCol - 1; > > + id = multi_select_coll_seq(parser, select, fieldno); > > + if (id != COLL_NONE) { > > + const char *name = coll_by_id(id)->name; > > + order_by->a[i].pExpr = > > + sqlExprAddCollateString(parser, expr, > > + name); > > + } > > + } > > + coll_ids[i] = id; > > + } > > + return 0; > > +} > > 8. It sligtly cuts the ears that a struct called fields_info also > defines a key via key_info. It is like specifying indexes inside of > space format. > > Maybe it is worth renaming it to ephemeral_space_def? Or space_info? > Or shorten ephemeral to ephmen in all the new code and use > ephem_space_def? Or sql_space_info? sql_ephemeral_space_def? > > And make sql_ephemeral_space_create() take this struct instead of > 2 arrays. Besides, you could hide sql_ephemeral_space_create_key_info() > inside of it because you could check key_info != null in > sql_ephemeral_space_create(). It just does not seem like vdbe business > to check these things. > Thank you! I renamed the structure to space_info. However, there are times when I include space_info in key_info (it was the other way around in the last patch). These are cases where the ephemeral space might be converted to SorterOpen opcodes or its key_info might change (see pushOntoSorter()). Actually, I tried to replace key_info everywhere, as there are two cases where the type of a field can change - when values are inserted into an ephemeral space and when a sorter is used. However, when I thought I could do this, I found it all pointless as there is no normal way to get data from the sorter. A "pseudo" cursor is used to get data from the sorter, and this cursor can only read tuples. So, after receiving data using a "pseudo" cursor, we lose information about the SCALAR and NUMBER field types. I think this is not as problematic as in the case of ephemeral space, where we got the SCALAR after selecting from ephemeral space. This is a bug, but since the sorter is rarely used, especially with the SCALAR field, I think we can fix this problem a little later. In the end, I decided to leave the opcodes that use key_info, other than OP_OpenTEphemeral, as they are now. > > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > > index ef8dcd693..167e80057 100644 > > --- a/src/box/sql/sqlInt.h > > +++ b/src/box/sql/sqlInt.h > > @@ -2248,6 +2248,11 @@ struct Parse { > > #define OPFLAG_SYSTEMSP 0x20 /* OP_Open**: set if space pointer > > * points to system space. > > */ > > +/** > > + * If this flag is set, than in OP_OpenTEphemeral we should use rowid as the > > 9. than -> then. Dropped. New patch: commit c445da3d3a55646b9029f2e749e8accb290a13ce Author: Mergen Imeev Date: Tue Aug 10 08:27:14 2021 +0300 sql: define ephemeral space fields Prior to this patch, most ephemeral space fields were defined using the SCALAR type. After this patch, almost all fields will be properly defined. However, there are still cases where SCALAR will be set by default. For example, in IN, where rules is still not defined (#4692). Another example is when a field is not resolved because of a too complex query. Part of #6213 diff --git a/src/box/sql.c b/src/box/sql.c index b87d236d1..d4b88d6e5 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -243,114 +243,153 @@ tarantoolsqlCount(struct BtCursor *pCur) return index_count(pCur->index, pCur->iter_type, NULL, 0); } -struct space * -sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) +struct space_info * +sql_space_info_new(uint32_t field_count, uint32_t part_count) { - struct key_def *def = NULL; - uint32_t part_count = field_count; - if (key_info != NULL) { - def = sql_key_info_to_key_def(key_info); - if (def == NULL) - return NULL; - /* - * In case is_pk_rowid is true we can use rowid - * as the only part of the key. - */ - if (key_info->is_pk_rowid) - part_count = 1; + assert(field_count > 0); + uint32_t info_size = sizeof(struct space_info); + uint32_t field_size = field_count * sizeof(enum field_type); + uint32_t colls_size = field_count * sizeof(uint32_t); + uint32_t parts_size = part_count * sizeof(uint32_t); + uint32_t size = info_size + field_size + colls_size + parts_size; + + struct space_info *info = sqlDbMallocRawNN(sql_get(), size); + if (info == NULL) { + diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "info"); + return NULL; } + info->types = (enum field_type *)((char *)info + info_size); + info->coll_ids = (uint32_t *)((char *)info->types + field_size); + info->parts = part_count == 0 ? NULL : + (uint32_t *)((char *)info->coll_ids + colls_size); + info->field_count = field_count; + info->part_count = part_count; + info->sort_order = SORT_ORDER_ASC; - struct region *region = &fiber()->gc; + for (uint32_t i = 0; i < field_count; ++i) { + info->types[i] = FIELD_TYPE_SCALAR; + info->coll_ids[i] = COLL_NONE; + } + for (uint32_t i = 0; i < part_count; ++i) + info->parts[i] = i; + return info; +} + +void +sql_space_info_from_space_def(struct space_info *info, + const struct space_def *def) +{ + assert(info->field_count == def->field_count + 1); + assert(info->part_count == 0); + for (uint32_t i = 0; i < def->field_count; ++i) { + info->types[i] = def->fields[i].type; + info->coll_ids[i] = def->fields[i].coll_id; + } + info->types[def->field_count] = FIELD_TYPE_INTEGER; + info->coll_ids[def->field_count] = COLL_NONE; +} + +void +sql_space_info_from_index_def(struct space_info *info, + const struct index_def *def) +{ + assert(info->field_count >= def->key_def->part_count); + assert(info->part_count == 0); + for (uint32_t i = 0; i < def->key_def->part_count; ++i) { + info->types[i] = def->key_def->parts[i].type; + info->coll_ids[i] = def->key_def->parts[i].coll_id; + } +} + +enum { /* - * Name of the fields will be "_COLUMN_1", "_COLUMN_2" - * and so on. Due to this, length of each name is no more - * than strlen("_COLUMN_") plus length of UINT32_MAX - * turned to string, which is 10 and plus 1 for \0. + * Name of the fields will be "_COLUMN_1", "_COLUMN_2" and so on. Due to + * this, length of each name is no more than strlen("_COLUMN_") plus + * length of UINT32_MAX turned to string, which is 10 and plus 1 for \0. */ - uint32_t name_len = strlen("_COLUMN_") + 11; - uint32_t size = field_count * (sizeof(struct field_def) + name_len) + - part_count * sizeof(struct key_part_def); + NAME_LEN = 19, +}; + +struct space * +sql_ephemeral_space_new_from_info(const struct space_info *info) +{ + uint32_t field_count = info->field_count; + uint32_t part_count = info->parts == NULL ? field_count : + info->part_count; + uint32_t parts_indent = field_count * sizeof(struct field_def); + uint32_t names_indent = part_count * sizeof(struct key_part_def) + + parts_indent; + uint32_t size = names_indent + field_count * NAME_LEN; + + struct region *region = &fiber()->gc; + size_t svp = region_used(region); struct field_def *fields = region_aligned_alloc(region, size, alignof(fields[0])); if (fields == NULL) { diag_set(OutOfMemory, size, "region_aligned_alloc", "fields"); return NULL; } - struct key_part_def *ephemer_key_parts = - (void *)fields + field_count * sizeof(struct field_def); - static_assert(alignof(*fields) == alignof(*ephemer_key_parts), - "allocated in one block, and should have the same " - "alignment"); - char *names = (char *)ephemer_key_parts + - part_count * sizeof(struct key_part_def); - for (uint32_t i = 0; i < field_count; ++i) { - struct field_def *field = &fields[i]; - field->name = names; - names += name_len; - sprintf(field->name, "_COLUMN_%d", i); - field->is_nullable = true; - field->nullable_action = ON_CONFLICT_ACTION_NONE; - field->default_value = NULL; - field->default_value_expr = NULL; - if (def != NULL && i < def->part_count) { - assert(def->parts[i].type < field_type_MAX); - field->type = def->parts[i].type; - field->coll_id = def->parts[i].coll_id; - } else { - field->coll_id = COLL_NONE; - field->type = FIELD_TYPE_SCALAR; - } + struct key_part_def *parts = (void *)fields + parts_indent; + static_assert(alignof(*fields) == alignof(*parts), "allocated in one " + "block, and should have the same alignment"); + char *names = (char *)fields + names_indent; + + for (uint32_t i = 0; i < info->field_count; ++i) { + fields[i].name = &names[i * NAME_LEN]; + sprintf(fields[i].name, "_COLUMN_%d", i); + fields[i].is_nullable = true; + fields[i].nullable_action = ON_CONFLICT_ACTION_NONE; + fields[i].default_value = NULL; + fields[i].default_value_expr = NULL; + fields[i].type = info->types[i]; + fields[i].coll_id = info->coll_ids[i]; + } + if (info->parts == NULL) { + for (uint32_t i = 0; i < part_count; ++i) + parts[i].fieldno = i; + } else { + for (uint32_t i = 0; i < part_count; ++i) + parts[i].fieldno = info->parts[i]; } - for (uint32_t i = 0; i < part_count; ++i) { - struct key_part_def *part = &ephemer_key_parts[i]; - /* - * In case we need to save initial order of - * inserted into ephemeral space rows we use rowid - * as the only part of PK. If ephemeral space has - * a rowid, it is always the last column. - */ - uint32_t j = i; - if (key_info != NULL && key_info->is_pk_rowid) - j = field_count - 1; - part->fieldno = j; - part->nullable_action = ON_CONFLICT_ACTION_NONE; - part->is_nullable = true; - part->exclude_null = false; - part->sort_order = SORT_ORDER_ASC; - part->path = NULL; - part->type = fields[j].type; - part->coll_id = fields[j].coll_id; + parts[i].nullable_action = ON_CONFLICT_ACTION_NONE; + parts[i].is_nullable = true; + parts[i].exclude_null = false; + parts[i].sort_order = info->sort_order; + parts[i].path = NULL; + parts[i].type = info->types[parts[i].fieldno]; + parts[i].coll_id = info->coll_ids[parts[i].fieldno]; } - struct key_def *ephemer_key_def = key_def_new(ephemer_key_parts, - part_count, false); - if (ephemer_key_def == NULL) + + struct key_def *key_def = key_def_new(parts, part_count, false); + if (key_def == NULL) return NULL; - struct index_def *ephemer_index_def = - index_def_new(0, 0, "ephemer_idx", strlen("ephemer_idx"), TREE, - &index_opts_default, ephemer_key_def, NULL); - key_def_delete(ephemer_key_def); - if (ephemer_index_def == NULL) + const char *name = "ephemer_idx"; + struct index_def *index_def = index_def_new(0, 0, name, strlen(name), + TREE, &index_opts_default, + key_def, NULL); + key_def_delete(key_def); + if (index_def == NULL) return NULL; struct rlist key_list; rlist_create(&key_list); - rlist_add_entry(&key_list, ephemer_index_def, link); + rlist_add_entry(&key_list, index_def, link); - struct space_def *ephemer_space_def = - space_def_new_ephemeral(field_count, fields); - if (ephemer_space_def == NULL) { - index_def_delete(ephemer_index_def); + struct space_def *space_def = space_def_new_ephemeral(field_count, + fields); + if (space_def == NULL) { + index_def_delete(index_def); return NULL; } - struct space *ephemer_new_space = space_new_ephemeral(ephemer_space_def, - &key_list); - index_def_delete(ephemer_index_def); - space_def_delete(ephemer_space_def); + struct space *space = space_new_ephemeral(space_def, &key_list); + index_def_delete(index_def); + space_def_delete(space_def); + region_truncate(region, svp); - return ephemer_new_space; + return space; } int tarantoolsqlEphemeralInsert(struct space *space, const char *tuple, diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 5226dd6ea..9d5a63fc5 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -224,12 +224,20 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, * is held in ephemeral table, there is no PK for * it, so columns should be loaded manually. */ - struct sql_key_info *pk_info = NULL; int reg_eph = ++parse->nMem; int reg_pk = parse->nMem + 1; - int pk_len; + int pk_len = is_view ? space->def->field_count + 1 : + space->index[0]->def->key_def->part_count; int eph_cursor = parse->nTab++; int addr_eph_open = sqlVdbeCurrentAddr(v); + struct space_info *info = sql_space_info_new(pk_len, 0); + if (info == NULL) { + parse->is_aborted = true; + goto delete_from_cleanup; + } + parse->nMem += pk_len; + sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, pk_len, 0, + (char *)info, P4_DYNAMIC); if (is_view) { /* * At this stage SELECT is already materialized @@ -249,21 +257,11 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, * account that id field as well. That's why pk_len * has one field more than view format. */ - pk_len = space->def->field_count + 1; - parse->nMem += pk_len; - sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph, - pk_len); + sql_space_info_from_space_def(info, space->def); } else { assert(space->index_count > 0); - pk_info = sql_key_info_new_from_key_def(db, - space->index[0]->def->key_def); - if (pk_info == NULL) - goto delete_from_cleanup; - pk_len = pk_info->part_count; - parse->nMem += pk_len; - sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, - pk_len, 0, - (char *)pk_info, P4_KEYINFO); + struct index_def *index_def = space->index[0]->def; + sql_space_info_from_index_def(info, index_def); } /* Construct a query to find the primary key for @@ -295,8 +293,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, /* Extract the primary key for the current row */ if (!is_view) { - struct key_part_def *part = pk_info->parts; - for (int i = 0; i < pk_len; i++, part++) { + struct key_def *def = space->index[0]->def->key_def; + for (int i = 0; i < pk_len; i++) { + struct key_part *part = &def->parts[i]; sqlVdbeAddOp3(v, OP_Column, tab_cursor, part->fieldno, reg_pk + i); } @@ -373,7 +372,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, if (one_pass != ONEPASS_OFF) { /* OP_Found will use an unpacked key. */ assert(key_len == pk_len); - assert(pk_info != NULL || space->def->opts.is_view); sqlVdbeAddOp4Int(v, OP_NotFound, tab_cursor, addr_bypass, reg_key, key_len); diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 6f40183ac..c67a7091c 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2740,7 +2740,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ switch (pExpr->op) { case TK_IN:{ - int addr; /* Address of OP_OpenEphemeral instruction */ Expr *pLeft = pExpr->pLeft; /* the LHS of the IN operator */ int nVal; /* Size of vector pLeft */ @@ -2761,13 +2760,13 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ */ pExpr->iTable = pParse->nTab++; int reg_eph = ++pParse->nMem; - addr = sqlVdbeAddOp2(v, OP_OpenTEphemeral, - reg_eph, nVal); + struct space_info *info = sql_space_info_new(nVal, 0); + if (info == NULL) + return 0; + sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nVal, 0, + (char *)info, P4_DYNAMIC); sqlVdbeAddOp3(v, OP_IteratorOpen, pExpr->iTable, 0, reg_eph); - struct sql_key_info *key_info = sql_key_info_new(pParse->db, nVal); - if (key_info == NULL) - return 0; if (ExprHasProperty(pExpr, EP_xIsSelect)) { /* Case 1: expr IN (SELECT ...) @@ -2797,7 +2796,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ (pParse, pSelect, &dest)) { sqlDbFree(pParse->db, dest.dest_type); - sql_key_info_unref(key_info); return 0; } sqlDbFree(pParse->db, @@ -2809,7 +2807,7 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ sqlVectorFieldSubexpr (pLeft, i); if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr, - &key_info->parts[i].coll_id) != 0) + &info->coll_ids[i]) != 0) return 0; } } @@ -2829,7 +2827,7 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ bool unused; struct coll *unused_coll; if (sql_expr_coll(pParse, pExpr->pLeft, &unused, - &key_info->parts[0].coll_id, + &info->coll_ids[0], &unused_coll) != 0) return 0; @@ -2861,8 +2859,6 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ sqlReleaseTempReg(pParse, r1); sqlReleaseTempReg(pParse, r2); } - sqlVdbeChangeP4(v, addr, (void *)key_info, - P4_KEYINFO); break; } diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 21b4f2407..99b5ceee6 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -442,34 +442,44 @@ sqlInsert(Parse * pParse, /* Parser context */ reg_eph = ++pParse->nMem; regRec = sqlGetTempReg(pParse); regCopy = sqlGetTempRange(pParse, nColumn + 1); - sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph, - nColumn + 1); /* - * This key_info is used to show that - * rowid should be the first part of PK in - * case we used AUTOINCREMENT feature. - * This way we will save initial order of - * the inserted values. The order is - * important if we use the AUTOINCREMENT - * feature, since changing the order can - * change the number inserted instead of - * NULL. + * Order of inserted values is important since it is + * possible, that NULL will be inserted in field with + * AUTOINCREMENT. So, the first part of key should be + * rowid. Since each rowid is unique, we do not need any + * other parts. */ - if (space->sequence != NULL) { - struct sql_key_info *key_info = - sql_key_info_new(pParse->db, - nColumn + 1); - key_info->parts[nColumn].type = - FIELD_TYPE_UNSIGNED; - key_info->is_pk_rowid = true; - sqlVdbeChangeP4(v, -1, (void *)key_info, - P4_KEYINFO); + struct space_info *info = + sql_space_info_new(nColumn + 1, 1); + if (info == NULL) { + pParse->is_aborted = true; + goto insert_cleanup; } + struct field_def *fields = space_def->fields; + if (pColumn != NULL) { + for (int i = 0; i < nColumn; ++i) { + int j = pColumn->a[i].idx; + info->types[i] = fields[j].type; + info->coll_ids[i] = fields[j].coll_id; + } + } else { + for (int i = 0; i < nColumn; ++i) { + info->types[i] = fields[i].type; + info->coll_ids[i] = fields[i].coll_id; + } + } + info->types[nColumn] = FIELD_TYPE_INTEGER; + info->parts[0] = nColumn; + + sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, + nColumn + 1, 0, (char *)info, P4_DYNAMIC); addrL = sqlVdbeAddOp1(v, OP_Yield, dest.iSDParm); VdbeCoverage(v); sqlVdbeAddOp2(v, OP_NextIdEphemeral, reg_eph, regCopy + nColumn); sqlVdbeAddOp3(v, OP_Copy, regFromSelect, regCopy, nColumn-1); + sqlVdbeAddOp4(v, OP_ApplyType, regCopy, nColumn + 1, 0, + (char *)info->types, P4_STATIC); sqlVdbeAddOp3(v, OP_MakeRecord, regCopy, nColumn + 1, regRec); /* Set flag to save memory allocating one by malloc. */ diff --git a/src/box/sql/select.c b/src/box/sql/select.c index b9107fccc..a26ca54a3 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -99,6 +99,74 @@ struct SortCtx { #define SORTFLAG_UseSorter 0x01 /* Use SorterOpen instead of OpenEphemeral */ #define SORTFLAG_DESC 0xF0 +static inline uint32_t +multi_select_coll_seq(struct Parse *parser, struct Select *p, int n); + +static int +space_info_from_expr_list(struct space_info *info, struct Parse *parser, + struct ExprList *list, int start) +{ + assert((int)info->field_count >= list->nExpr - start); + for (int i = 0; i < list->nExpr - start; ++i) { + bool b; + struct coll *coll; + struct Expr *expr = list->a[i + start].pExpr; + enum field_type type = sql_expr_type(expr); + /* + * Type ANY could mean that field was unresolved. We have no way + * but to set it as SCALAR, however this could lead to + * unexpected change of type. + */ + if (type == FIELD_TYPE_ANY) + type = FIELD_TYPE_SCALAR; + uint32_t coll_id; + if (sql_expr_coll(parser, expr, &b, &coll_id, &coll) != 0) + return -1; + info->types[i] = type; + info->coll_ids[i] = coll_id; + } + info->sort_order = list->a[0].sort_order; + return 0; +} + +static int +space_info_from_order_by(struct space_info *info, struct Parse *parser, + struct Select *select, struct ExprList *list) +{ + for (int i = 0; i < list->nExpr; ++i) { + bool b; + struct Expr *expr = list->a[i].pExpr; + enum field_type type = sql_expr_type(expr); + /* + * Type ANY could mean that field was unresolved. We have no way + * but to set it as SCALAR, however this could lead to + * unexpected change of type. + */ + if (type == FIELD_TYPE_ANY) + type = FIELD_TYPE_SCALAR; + info->types[i] = type; + uint32_t *id = &info->coll_ids[i]; + if ((expr->flags & EP_Collate) != 0) { + struct coll *coll; + if (sql_expr_coll(parser, expr, &b, id, &coll) != 0) + return -1; + continue; + } + uint32_t fieldno = list->a[i].u.x.iOrderByCol - 1; + info->coll_ids[i] = multi_select_coll_seq(parser, select, + fieldno); + if (info->coll_ids[i] != COLL_NONE) { + const char *name = coll_by_id(info->coll_ids[i])->name; + list->a[i].pExpr = + sqlExprAddCollateString(parser, expr, name); + } + } + info->sort_order = list->a[0].sort_order; + info->types[list->nExpr] = FIELD_TYPE_INTEGER; + info->types[list->nExpr + 1] = FIELD_TYPE_VARBINARY; + return 0; +} + /* * Delete all the content of a Select structure. Deallocate the structure * itself only if bFree is true. @@ -823,9 +891,29 @@ pushOntoSorter(Parse * pParse, /* Parser context */ for (uint32_t i = 0; i < key_info->part_count; i++) key_info->parts[i].sort_order = SORT_ORDER_ASC; sqlVdbeChangeP4(v, -1, (char *)key_info, P4_KEYINFO); - pOp->p4.key_info = sql_expr_list_to_key_info(pParse, - pSort->pOrderBy, - nOBSat); + assert(pOp->opcode == OP_OpenTEphemeral || + pOp->opcode == OP_SorterOpen); + if (pOp->opcode == OP_SorterOpen) { + pOp->p4.key_info = + sql_expr_list_to_key_info(pParse, + pSort->pOrderBy, + nOBSat); + } else { + struct space_info *info = + sql_space_info_new(pOp->p2, 0); + if (info == NULL) { + pParse->is_aborted = true; + return; + } + pOp->p4.space_info = info; + pOp->p4type = P4_DYNAMIC; + if (space_info_from_expr_list(info, pParse, + pSort->pOrderBy, + nOBSat) != 0) { + pParse->is_aborted = true; + return; + } + } addrJmp = sqlVdbeCurrentAddr(v); sqlVdbeAddOp3(v, OP_Jump, addrJmp + 1, 0, addrJmp + 1); VdbeCoverage(v); @@ -1044,13 +1132,28 @@ selectInnerLoop(Parse * pParse, /* The parser context */ * space format. */ uint32_t excess_field_count = 0; + struct VdbeOp *op = sqlVdbeGetOp(v, + pSort->addrSortIndex); + struct space_info *info; + if (op->p4type == P4_KEYINFO) + info = op->p4.key_info->info; + else + info = op->p4.space_info; for (i = pSort->nOBSat; i < pSort->pOrderBy->nExpr; i++) { int j = pSort->pOrderBy->a[i].u.x.iOrderByCol; - if (j > 0) { - excess_field_count++; - pEList->a[j - 1].u.x.iOrderByCol = - (u16) (i + 1 - pSort->nOBSat); + if (j == 0) + continue; + assert(j > 0); + excess_field_count++; + pEList->a[j - 1].u.x.iOrderByCol = + (u16) (i + 1 - pSort->nOBSat); + --info->field_count; + for (int k = j; k < pEList->nExpr; ++k) { + int n = k + pSort->pOrderBy->nExpr + 1; + info->types[n - 1] = info->types[n]; + info->coll_ids[n - 1] = + info->coll_ids[n]; } } struct VdbeOp *open_eph_op = @@ -1416,6 +1519,7 @@ sql_key_info_new(sql *db, uint32_t part_count) return NULL; } key_info->db = db; + key_info->info = NULL; key_info->key_def = NULL; key_info->refs = 1; key_info->part_count = part_count; @@ -1444,6 +1548,7 @@ sql_key_info_new_from_key_def(sql *db, const struct key_def *key_def) return NULL; } key_info->db = db; + key_info->info = NULL; key_info->key_def = NULL; key_info->refs = 1; key_info->part_count = key_def->part_count; @@ -1469,6 +1574,7 @@ sql_key_info_unref(struct sql_key_info *key_info) if (--key_info->refs == 0) { if (key_info->key_def != NULL) key_def_delete(key_info->key_def); + sqlDbFree(key_info->db, key_info->info); sqlDbFree(key_info->db, key_info); } } @@ -2453,17 +2559,29 @@ generateWithRecursiveQuery(Parse * pParse, /* Parsing context */ /* Allocate cursors for Current, Queue, and Distinct. */ regCurrent = ++pParse->nMem; sqlVdbeAddOp3(v, OP_OpenPseudo, iCurrent, regCurrent, nCol); + int field_count = pOrderBy != NULL ? pOrderBy->nExpr + 2 : nCol + 1; + struct space_info *info = sql_space_info_new(field_count, 0); + if (info == NULL) { + pParse->is_aborted = true; + goto end_of_recursive_query; + } + sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, field_count, 0, + (char *)info, P4_DYNAMIC); if (pOrderBy) { - struct sql_key_info *key_info = - sql_multiselect_orderby_to_key_info(pParse, p, 1); - sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_queue, - pOrderBy->nExpr + 2, 0, (char *)key_info, - P4_KEYINFO); VdbeComment((v, "Orderby table")); + if (space_info_from_order_by(info, pParse, p, pOrderBy) != 0) { + pParse->is_aborted = true; + goto end_of_recursive_query; + } destQueue.pOrderBy = pOrderBy; } else { - sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_queue, nCol + 1); VdbeComment((v, "Queue table")); + if (space_info_from_expr_list(info, pParse, p->pEList, + 0) != 0) { + pParse->is_aborted = true; + goto end_of_recursive_query; + } + info->types[nCol] = FIELD_TYPE_INTEGER; } sqlVdbeAddOp3(v, OP_IteratorOpen, iQueue, 0, reg_queue); if (iDistinct) { @@ -2673,7 +2791,21 @@ multiSelect(Parse * pParse, /* Parsing context */ if (dest.eDest == SRT_EphemTab) { assert(p->pEList); int nCols = p->pEList->nExpr; - sqlVdbeAddOp2(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1); + struct space_info *info = sql_space_info_new(nCols + 1, 0); + if (info == NULL) { + pParse->is_aborted = true; + rc = 1; + goto multi_select_end; + } + sqlVdbeAddOp4(v, OP_OpenTEphemeral, dest.reg_eph, nCols + 1, + 0, (char *)info, P4_DYNAMIC); + if (space_info_from_expr_list(info, pParse, p->pEList, + 0) != 0) { + pParse->is_aborted = true; + rc = 1; + goto multi_select_end; + } + info->types[nCols] = FIELD_TYPE_INTEGER; sqlVdbeAddOp3(v, OP_IteratorOpen, dest.iSDParm, 0, dest.reg_eph); VdbeComment((v, "Destination temp")); dest.eDest = SRT_Table; @@ -3001,14 +3133,14 @@ multiSelect(Parse * pParse, /* Parsing context */ if (p->selFlags & SF_UsesEphemeral) { assert(p->pNext == NULL); int nCol = p->pEList->nExpr; - struct sql_key_info *key_info = sql_key_info_new(db, nCol); - if (key_info == NULL) + struct space_info *info = sql_space_info_new(nCol, 0); + if (info == NULL) { + pParse->is_aborted = true; goto multi_select_end; - for (int i = 0; i < nCol; i++) { - key_info->parts[i].coll_id = - multi_select_coll_seq(pParse, p, i); } - + for (int i = 0; i < nCol; ++i) + info->coll_ids[i] = multi_select_coll_seq(pParse, p, i); + bool is_info_used = false; for (struct Select *pLoop = p; pLoop; pLoop = pLoop->pPrior) { for (int i = 0; i < 2; i++) { int addr = pLoop->addrOpenEphm[i]; @@ -3020,13 +3152,15 @@ multiSelect(Parse * pParse, /* Parsing context */ break; } sqlVdbeChangeP2(v, addr, nCol); - sqlVdbeChangeP4(v, addr, - (char *)sql_key_info_ref(key_info), - P4_KEYINFO); + sqlVdbeChangeP4(v, addr, (char *)info, + is_info_used ? + P4_STATIC : P4_DYNAMIC); + is_info_used = true; pLoop->addrOpenEphm[i] = -1; } } - sql_key_info_unref(key_info); + if (!is_info_used) + sqlDbFree(pParse->db, info); } multi_select_end: @@ -5347,17 +5481,31 @@ resetAccumulator(Parse * pParse, AggInfo * pAggInfo) "exactly one argument"); pParse->is_aborted = true; pFunc->iDistinct = -1; - } else { - struct sql_key_info *key_info = - sql_expr_list_to_key_info(pParse, - pE->x.pList, - 0); - sqlVdbeAddOp4(v, OP_OpenTEphemeral, - pFunc->reg_eph, 1, 0, - (char *)key_info, P4_KEYINFO); - sqlVdbeAddOp3(v, OP_IteratorOpen, - pFunc->iDistinct, 0, pFunc->reg_eph); + return; + } + assert(pE->x.pList->nExpr == 1); + struct space_info *info = sql_space_info_new(1, 0); + if (info == NULL) { + pParse->is_aborted = true; + pFunc->iDistinct = -1; + return; } + sqlVdbeAddOp4(v, OP_OpenTEphemeral, pFunc->reg_eph, 1, + 0, (char *)info, P4_DYNAMIC); + bool b; + struct coll *coll; + struct Expr *expr = pE->x.pList->a[0].pExpr; + info->types[0] = sql_expr_type(expr); + if (info->types[0] == FIELD_TYPE_ANY) + info->types[0] = FIELD_TYPE_SCALAR; + if (sql_expr_coll(pParse, expr, &b, &info->coll_ids[0], + &coll) != 0) { + pParse->is_aborted = true; + pFunc->iDistinct = -1; + return; + } + sqlVdbeAddOp3(v, OP_IteratorOpen, pFunc->iDistinct, 0, + pFunc->reg_eph); } } } @@ -5879,6 +6027,39 @@ sqlSelect(Parse * pParse, /* The parser context */ sSort.reg_eph, nCols, 0, (char *)key_info, P4_KEYINFO); + /* + * We also should define space_info in case this opcode will not + * be converted to SorterOpen. + */ + uint32_t part_count = sSort.pOrderBy->nExpr + 1; + struct space_info *info = sql_space_info_new(nCols, part_count); + if (info == NULL) { + pParse->is_aborted = true; + goto select_end; + } + key_info->info = info; + struct ExprList *list = sSort.pOrderBy; + if (space_info_from_expr_list(info, pParse, list, 0) != 0) { + pParse->is_aborted = true; + goto select_end; + } + info->types[sSort.pOrderBy->nExpr] = FIELD_TYPE_INTEGER; + for (int i = 0; i < pEList->nExpr; ++i) { + bool b; + struct Expr *expr = pEList->a[i].pExpr; + enum field_type type = sql_expr_type(expr); + if (type == FIELD_TYPE_ANY) + type = FIELD_TYPE_SCALAR; + uint32_t id; + struct coll *coll; + if (sql_expr_coll(pParse, expr, &b, &id, &coll) != 0) { + pParse->is_aborted = true; + goto select_end; + } + int j = part_count + i; + info->types[j] = type; + info->coll_ids[j] = id; + } sqlVdbeAddOp3(v, OP_IteratorOpen, sSort.iECursor, 0, sSort.reg_eph); VdbeComment((v, "Sort table")); } else { @@ -5888,11 +6069,19 @@ sqlSelect(Parse * pParse, /* The parser context */ /* If the output is destined for a temporary table, open that table. */ if (pDest->eDest == SRT_EphemTab) { - struct sql_key_info *key_info = - sql_expr_list_to_key_info(pParse, pEList, 0); - sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, - pEList->nExpr + 1, 0, (char *)key_info, - P4_KEYINFO); + uint32_t cols = pEList->nExpr + 1; + struct space_info *info = sql_space_info_new(cols, 0); + if (info == NULL) { + pParse->is_aborted = true; + goto select_end; + } + sqlVdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph, cols, 0, + (char *)info, P4_DYNAMIC); + if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) { + pParse->is_aborted = true; + goto select_end; + } + info->types[pEList->nExpr] = FIELD_TYPE_INTEGER; sqlVdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0, pDest->reg_eph); @@ -5918,13 +6107,19 @@ sqlSelect(Parse * pParse, /* The parser context */ if (p->selFlags & SF_Distinct) { sDistinct.cur_eph = pParse->nTab++; sDistinct.reg_eph = ++pParse->nMem; - struct sql_key_info *key_info = - sql_expr_list_to_key_info(pParse, p->pEList, 0); + uint32_t cols = pEList->nExpr; + struct space_info *info = sql_space_info_new(cols, 0); + if (info == NULL) { + pParse->is_aborted = true; + goto select_end; + } sDistinct.addrTnct = sqlVdbeAddOp4(v, OP_OpenTEphemeral, - sDistinct.reg_eph, - key_info->part_count, - 0, (char *)key_info, - P4_KEYINFO); + sDistinct.reg_eph, cols, 0, + (char *)info, P4_DYNAMIC); + if (space_info_from_expr_list(info, pParse, pEList, 0) != 0) { + pParse->is_aborted = true; + goto select_end; + } sqlVdbeAddOp3(v, OP_IteratorOpen, sDistinct.cur_eph, 0, sDistinct.reg_eph); VdbeComment((v, "Distinct table")); diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 1f87e6823..60fa1678d 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4006,6 +4006,8 @@ sql_analysis_load(struct sql *db); */ struct sql_key_info { sql *db; + /* Informations about all field types and key parts. */ + struct space_info *info; /** * Key definition created from this object, * see sql_key_info_to_key_def(). @@ -4055,6 +4057,66 @@ sql_key_info_unref(struct sql_key_info *key_info); struct key_def * sql_key_info_to_key_def(struct sql_key_info *key_info); +/** + * Structure that is used to store information about ephemeral space field types + * and fieldno of key parts. + */ +struct space_info { + /** Field types of all fields of ephemeral space. */ + enum field_type *types; + /** Collation ids of all fields of ephemeral space. */ + uint32_t *coll_ids; + /** + * Fieldno key parts of the ephemeral space. If NULL, then the index + * consists of all fields in sequential order. + */ + uint32_t *parts; + /** Number of fields of ephemetal space. */ + uint32_t field_count; + /** + * Number of parts in primary index of ephemetal space. If 0 then parts + * is also NULL. + */ + uint32_t part_count; + /** Sort order of index. */ + enum sort_order sort_order; +}; + +/** + * Allocate and initialize with default values a structure that will be used to + * store information about ephemeral space field types and key parts. + */ +struct space_info * +sql_space_info_new(uint32_t field_count, uint32_t part_count); + +/** + * Initialize the field types and key parts of space_info with space_def. + * Additionally added one more field type and key part for rowid. Rowid is + * always INTEGER. Key parts will be initialized with the same values as the + * field types. The number of initialized field types and key parts will be the + * same as the field_count in space_def plus one. + */ +void +sql_space_info_from_space_def(struct space_info *info, + const struct space_def *def); + +/** + * Initialize the field types and key parts of space_info with index_def. + * Key parts will be initialized with the same values as the field types. The + * number of initialized field types and key parts will be the same as the + * part_count in index_def. + */ +void +sql_space_info_from_index_def(struct space_info *info, + const struct index_def *def); + +/** + * Allocate and initialize an ephemeral space. Information about field types and + * key parts is taken from the space_info structure. + */ +struct space * +sql_ephemeral_space_new_from_info(const struct space_info *info); + /** * Check if the function implements LIKE-style comparison & if it * is appropriate to apply a LIKE query optimization. diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index 8fdc50432..adb7b5f9f 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -61,21 +61,6 @@ sql_rename_table(uint32_t space_id, const char *new_name); int tarantoolsqlRenameTrigger(const char *zTriggerName, const char *zOldName, const char *zNewName); -/** - * Create ephemeral space. Features of ephemeral spaces: id == 0, - * name == "ephemeral", memtx engine (in future it can be changed, - * but now only memtx engine is supported), primary index which - * covers all fields and no secondary indexes, given field number - * and collation sequence. All fields are scalar and nullable. - * - * @param field_count Number of fields in ephemeral space. - * @param key_info Keys description for new ephemeral space. - * - * @retval Pointer to created space, NULL if error. - */ -struct space * -sql_ephemeral_space_create(uint32_t filed_count, struct sql_key_info *key_info); - /** * Insert tuple into ephemeral space. * In contrast to ordinary spaces, there is no need to create and diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 22f82390c..8eab8a3df 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -226,9 +226,21 @@ sqlUpdate(Parse * pParse, /* The parser context */ iEph = pParse->nTab++; sqlVdbeAddOp2(v, OP_Null, 0, iPk); + struct space_info *info = sql_space_info_new(pk_part_count, 0); + if (info == NULL) { + pParse->is_aborted = true; + goto update_cleanup; + } /* Address of the OpenEphemeral instruction. */ - int addrOpen = sqlVdbeAddOp2(v, OP_OpenTEphemeral, reg_eph, - pk_part_count); + int addrOpen = sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, + pk_part_count, 0, (char *)info, + P4_DYNAMIC); + assert(space->index_count > 0 || is_view); + if (is_view) + sql_space_info_from_space_def(info, def); + else + sql_space_info_from_index_def(info, pPk->def); + pWInfo = sqlWhereBegin(pParse, pTabList, pWhere, 0, 0, WHERE_ONEPASS_DESIRED, pk_cursor); if (pWInfo == 0) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index fcea9eefe..14caa6f82 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2033,10 +2033,9 @@ case OP_Fetch: { case OP_ApplyType: { enum field_type *types = pOp->p4.types; assert(types != NULL); - assert(types[pOp->p2] == field_type_MAX); pIn1 = &aMem[pOp->p1]; - enum field_type type; - while((type = *(types++)) != field_type_MAX) { + for (int i = 0; i < pOp->p2; ++i, ++pIn1) { + enum field_type type = types[i]; assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]); assert(memIsValid(pIn1)); if (mem_cast_implicit(pIn1, type) != 0) { @@ -2044,7 +2043,6 @@ case OP_ApplyType: { mem_str(pIn1), field_type_strs[type]); goto abort_due_to_error; } - pIn1++; } break; } @@ -2392,10 +2390,11 @@ open_cursor_set_hints: case OP_OpenTEphemeral: { assert(pOp->p1 >= 0); assert(pOp->p2 > 0); - assert(pOp->p4type != P4_KEYINFO || pOp->p4.key_info != NULL); - struct space *space = sql_ephemeral_space_create(pOp->p2, - pOp->p4.key_info); + struct space_info *info = pOp->p4type == P4_KEYINFO ? + pOp->p4.key_info->info : pOp->p4.space_info; + assert(info != NULL); + struct space *space = sql_ephemeral_space_new_from_info(info); if (space == NULL) goto abort_due_to_error; diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index be112c72d..505892861 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -93,6 +93,10 @@ struct VdbeOp { * doing a cast. */ enum field_type *types; + /** + * Information about ephemeral space field types and key parts. + */ + struct space_info *space_info; } p4; #ifdef SQL_ENABLE_EXPLAIN_COMMENTS char *zComment; /* Comment to improve readability */ @@ -210,15 +214,6 @@ int sqlVdbeDeletePriorOpcode(Vdbe *, u8 op); void sqlVdbeChangeP4(Vdbe *, int addr, const char *zP4, int N); void sqlVdbeAppendP4(Vdbe *, void *pP4, int p4type); -/** - * Set the P4 on the most recently added opcode to the key_def for the - * index given. - * @param Parse context, for error reporting. - * @param key_def Definition of a key to set. - */ -void -sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def); - VdbeOp *sqlVdbeGetOp(Vdbe *, int); int sqlVdbeMakeLabel(Vdbe *); void sqlVdbeRunOnlyOnce(Vdbe *); diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 61be7b489..2d7800b17 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -787,18 +787,6 @@ sqlVdbeAppendP4(Vdbe * p, void *pP4, int n) } } -void -sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def) -{ - struct Vdbe *v = parse->pVdbe; - assert(v != NULL); - assert(key_def != NULL); - struct sql_key_info *key_info = - sql_key_info_new_from_key_def(parse->db, key_def); - if (key_info != NULL) - sqlVdbeAppendP4(v, key_info, P4_KEYINFO); -} - #ifdef SQL_ENABLE_EXPLAIN_COMMENTS /* * Change the comment on the most recently coded instruction. Or diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 16766f2f8..31490a9b2 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -919,15 +919,16 @@ constructAutomaticIndex(Parse * pParse, /* The parsing context */ /* Create the automatic index */ assert(pLevel->iIdxCur >= 0); pLevel->iIdxCur = pParse->nTab++; - struct sql_key_info *pk_info = - sql_key_info_new_from_key_def(pParse->db, idx_def->key_def); - if (pk_info == NULL) { + struct space_info *info = sql_space_info_new(nKeyCol + 1, 0); + if (info == NULL) { pParse->is_aborted = true; return; } int reg_eph = sqlGetTempReg(pParse); sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_eph, nKeyCol + 1, 0, - (char *)pk_info, P4_KEYINFO); + (char *)info, P4_DYNAMIC); + sql_space_info_from_index_def(info, idx_def); + info->types[nKeyCol] = FIELD_TYPE_INTEGER; sqlVdbeAddOp3(v, OP_IteratorOpen, pLevel->iIdxCur, 0, reg_eph); VdbeComment((v, "for %s", space->def->name)); diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index df6cc92e1..a8a1893d3 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1134,11 +1134,19 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about the W if ((pWInfo->wctrlFlags & WHERE_DUPLICATES_OK) == 0) { cur_row_set = pParse->nTab++; reg_row_set = ++pParse->nMem; - sqlVdbeAddOp2(v, OP_OpenTEphemeral, - reg_row_set, pk_part_count); + struct space_info *info = + sql_space_info_new(pk_part_count, 0); + if (info == NULL) { + pParse->is_aborted = true; + return notReady; + } + sqlVdbeAddOp4(v, OP_OpenTEphemeral, reg_row_set, + pk_part_count, 0, (char *)info, + P4_DYNAMIC); + struct index_def *index_def = space->index[0]->def; + sql_space_info_from_index_def(info, index_def); sqlVdbeAddOp3(v, OP_IteratorOpen, cur_row_set, 0, reg_row_set); - sql_vdbe_set_p4_key_def(pParse, pk_key_def); regPk = ++pParse->nMem; } iRetInit = sqlVdbeAddOp2(v, OP_Integer, 0, regReturn);