[Tarantool-patches] [PATCH v2 1/2] sql: specify field types in ephemeral space format

[Nikita Pettik]

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.

> +	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?

> +	struct key_part_def *ephemer_key_parts = (void *)fields +
> +				     field_count * sizeof(struct field_def);

Strange indentation.

> +	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.