From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (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 380564696C3 for ; Wed, 15 Apr 2020 10:09:43 +0300 (MSK) From: imeevma@tarantool.org Date: Wed, 15 Apr 2020 10:09:41 +0300 Message-Id: <7bd2ab4f48e1cdbd31bb273252849782b45e2df3.1586933931.git.imeevma@gmail.com> In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v3 1/3] box: extend ephemeral space format List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: korablev@tarantool.org, tsafin@tarantool.org, tarantool-patches@dev.tarantool.org Hi! Thank you for the review. My answers and new patch below. On 4/14/20 1:47 AM, Nikita Pettik wrote: > On 12 Apr 19:29, imeevma@tarantool.org wrote: >> diff --git a/src/box/sql.c b/src/box/sql.c >> index 1256df8..e4434b2 100644 >> --- a/src/box/sql.c >> +++ b/src/box/sql.c >> @@ -330,13 +330,41 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) >> return NULL; >> } >> >> - struct key_part_def *ephemer_key_parts = region_alloc(&fiber()->gc, >> - sizeof(*ephemer_key_parts) * field_count); >> - if (ephemer_key_parts == NULL) { >> - diag_set(OutOfMemory, sizeof(*ephemer_key_parts) * field_count, >> - "region", "key parts"); >> - return NULL; >> + struct region *region = &fiber()->gc; >> + /* >> + * Name of the fields will be "_COLUMN_1", "_COLUMN_2" >> + * and so on. Since number of columns no more than 2000, >> + * length of each name is no more than strlen("_COLUMN_") >> + * + 5. >> + */ >> + assert(SQL_MAX_COLUMN <= 2000); > > Ephemeral space is capable of storing more columns than casual table. > For instance eph. table holding result of join operations features > number of columns which equals to sum of all table's columns > participating in join. > Fixed. Now insted of 4 figures, we may use 10. >> + uint32_t name_len = strlen("_COLUMN_") + 5; >> + uint32_t size = field_count * (sizeof(struct field_def) + name_len + >> + sizeof(struct key_part_def)); >> + struct field_def *fields = region_alloc(region, size); > > NULL check? > Fixed. >> + struct key_part_def *ephemer_key_parts = (void *)fields + >> + field_count * sizeof(struct field_def); > > Strange indentation. > Fixed. >> + char *names = (char *)ephemer_key_parts + >> + field_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; >> + } >> } >> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c >> index 312c966..beaeb0f 100644 >> --- a/src/box/tuple_format.c >> +++ b/src/box/tuple_format.c >> @@ -698,6 +698,12 @@ tuple_format_destroy(struct tuple_format *format) >> * dictionary will never be altered. If it can, then alter can >> * affect another space, which shares a format with one which is >> * altered. >> + * >> + * The only way to change the format of the space is to recreate >> + * space with the new format inside of BOX. Since there is no >> + * mechanism for recreating the ephemeral space, we need not worry >> + * about changing the format of the ephemeral space. >> + * >> * @param p_format Double pointer to format. It is updated with >> * hashed value, if corresponding format was found >> * in hash table >> @@ -709,13 +715,7 @@ static bool >> tuple_format_reuse(struct tuple_format **p_format) >> { >> struct tuple_format *format = *p_format; >> - if (!format->is_ephemeral) >> - return false; >> - /* >> - * These fields do not participate in hashing. >> - * Make sure they're unset. >> - */ >> - assert(format->dict->name_count == 0); >> + assert(format->is_ephemeral); >> assert(format->is_temporary); >> mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, >> NULL); > > Personally I'd split this patch into two: one which affects box > component, and another one which introduces format of ephemeral > spaces in SQL. > Fixed. New patch: >From 7bd2ab4f48e1cdbd31bb273252849782b45e2df3 Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Tue, 14 Apr 2020 23:37:29 +0300 Subject: [PATCH] box: extend ephemeral space format This patch allows to set field types and names in ephemeral space formats. Needed for #4256 Needed for #4692 Part of #3841 diff --git a/src/box/space_def.c b/src/box/space_def.c index 6611312..0ff51b9 100644 --- a/src/box/space_def.c +++ b/src/box/space_def.c @@ -233,17 +233,21 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count, } struct space_def* -space_def_new_ephemeral(uint32_t field_count) +space_def_new_ephemeral(uint32_t exact_field_count, struct field_def *fields) { struct space_opts opts = space_opts_default; opts.is_temporary = true; opts.is_ephemeral = true; - struct space_def *space_def = space_def_new(0, 0, field_count, + uint32_t field_count = exact_field_count; + if (fields == NULL) { + fields = (struct field_def *)&field_def_default; + field_count = 0; + } + struct space_def *space_def = space_def_new(0, 0, exact_field_count, "ephemeral", strlen("ephemeral"), "memtx", strlen("memtx"), - &opts, &field_def_default, - 0); + &opts, fields, field_count); return space_def; } diff --git a/src/box/space_def.h b/src/box/space_def.h index ac6d226..788b601 100644 --- a/src/box/space_def.h +++ b/src/box/space_def.h @@ -172,12 +172,13 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count, /** * Create a new ephemeral space definition. - * @param field_count Number of fields in the space. + * @param exact_field_count Number of fields in the space. + * @param fields Field definitions. * * @retval Space definition. */ struct space_def * -space_def_new_ephemeral(uint32_t field_count); +space_def_new_ephemeral(uint32_t exact_field_count, struct field_def *fields); /** * Size of the space_def. diff --git a/src/box/sql.c b/src/box/sql.c index 6afb308..f6afb86 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -364,7 +364,7 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) rlist_add_entry(&key_list, ephemer_index_def, link); struct space_def *ephemer_space_def = - space_def_new_ephemeral(field_count); + space_def_new_ephemeral(field_count, NULL); if (ephemer_space_def == NULL) { index_def_delete(ephemer_index_def); return NULL; diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index 312c966..beaeb0f 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -698,6 +698,12 @@ tuple_format_destroy(struct tuple_format *format) * dictionary will never be altered. If it can, then alter can * affect another space, which shares a format with one which is * altered. + * + * The only way to change the format of the space is to recreate + * space with the new format inside of BOX. Since there is no + * mechanism for recreating the ephemeral space, we need not worry + * about changing the format of the ephemeral space. + * * @param p_format Double pointer to format. It is updated with * hashed value, if corresponding format was found * in hash table @@ -709,13 +715,7 @@ static bool tuple_format_reuse(struct tuple_format **p_format) { struct tuple_format *format = *p_format; - if (!format->is_ephemeral) - return false; - /* - * These fields do not participate in hashing. - * Make sure they're unset. - */ - assert(format->dict->name_count == 0); + assert(format->is_ephemeral); assert(format->is_temporary); mh_int_t key = mh_tuple_format_find(tuple_formats_hash, format, NULL); @@ -739,9 +739,7 @@ tuple_format_reuse(struct tuple_format **p_format) static int tuple_format_add_to_hash(struct tuple_format *format) { - if(!format->is_ephemeral) - return 0; - assert(format->dict->name_count == 0); + assert(format->is_ephemeral); assert(format->is_temporary); mh_int_t key = mh_tuple_format_put(tuple_formats_hash, (const struct tuple_format **)&format, @@ -795,11 +793,11 @@ tuple_format_new(struct tuple_format_vtab *vtab, void *engine, if (tuple_format_create(format, keys, key_count, space_fields, space_field_count) < 0) goto err; - if (tuple_format_reuse(&format)) + if (is_ephemeral && tuple_format_reuse(&format)) return format; if (tuple_format_register(format) < 0) goto err; - if (tuple_format_add_to_hash(format) < 0) { + if (is_ephemeral && tuple_format_add_to_hash(format) < 0) { tuple_format_deregister(format); goto err; }