Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: imeevma@tarantool.org
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 1/2] sql: specify field types in ephemeral space format
Date: Mon, 13 Apr 2020 22:47:31 +0000	[thread overview]
Message-ID: <20200413224731.GE24818@tarantool.org> (raw)
In-Reply-To: <c5fb9dd0fc114fe60184a653726c38a9e50c3c26.1586708735.git.imeevma@gmail.com>

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.

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

  reply	other threads:[~2020-04-13 22:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-12 16:29 [Tarantool-patches] [PATCH v2 0/2] sql: fix order of inserted rows imeevma
2020-04-12 16:29 ` [Tarantool-patches] [PATCH v2 1/2] sql: specify field types in ephemeral space format imeevma
2020-04-13 22:47   ` Nikita Pettik [this message]
2020-04-12 16:29 ` [Tarantool-patches] [PATCH v2 2/2] sql: do not change order of inserted values imeevma
2020-04-13 22:59   ` Nikita Pettik
2020-04-13 22:35 ` [Tarantool-patches] [PATCH v2 0/2] sql: fix order of inserted rows Nikita Pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200413224731.GE24818@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 1/2] sql: specify field types in ephemeral space format' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox