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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the fixes!

See 2 comments below.

> diff --git a/src/box/sql.c b/src/box/sql.c
> index 27089bd..288c4fa 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -337,14 +337,15 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
>  	 * length of each name is no more than strlen("_COLUMN_")
>  	 * + 5.
>  	 */
> -	assert(SQL_MAX_COLUMN < 9999);
> +	assert(SQL_MAX_COLUMN <= 2000);
>  	uint32_t name_len = sizeof("_COLUMN_") + 5;

1. sizeof("_COLUMN_") != strlen("_COLUMN_"). The latter is 8,
the former is 9. Better use strlen().

>  	uint32_t size = field_count * (sizeof(struct field_def) + name_len +
>  				       sizeof(struct key_part_def));
>  	struct field_def *fields = region_alloc(region, size);
> -	struct key_part_def *parts = (void *)fields +
> +	struct key_part_def *ephemer_key_parts = (void *)fields +
>  				     field_count * sizeof(struct field_def);
> -	char *names = (char *)parts + field_count * sizeof(struct key_part_def);
> +	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;

2. It is worth extending the commit message with more details, how
is it needed for 4256 and 4692, with all what we discussed in private.

Please, do the things above and send to Nikita.

Also, in case it will be pushed in this form to master, I propose you to
create a ticket on ephemeral_space API rework. As you fairly noticed, we
have issues with using sql_key_info as, in fact, a format definition.
We basically *can't* index fields not added to the format.