[Tarantool-patches] [PATCH v3 1/3] box: extend ephemeral space format
imeevma at tarantool.org
imeevma at tarantool.org
Wed Apr 15 10:09:41 MSK 2020
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 at 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 <imeevma at gmail.com>
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;
}
More information about the Tarantool-patches
mailing list