Re: [Tarantool-patches] [PATCH 10/14] WIP: module api: add box_key_def_dump_parts()

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

Thanks for the patch!

See 3 comments below.

On 23.09.2020 03:14, Alexander Turenko wrote:
> The function dumps an opacue <box_key_def_t> structure into a non-opacue

1. opacue -> opaque.

> array of <box_key_part_def_t> structures in order to allow an external
> module to obtain information about the key definition.

2. Why would a module need it, if it already had all the parameters used
to create this key def?

> XXX: Add a module API test.
> 
> Part of #5273
> ---
>  src/box/key_def_api.c | 64 +++++++++++++++++++++++++++++++++++++++++++
>  src/box/key_def_api.h | 10 +++++++
>  src/exports.h         |  1 +
>  3 files changed, 75 insertions(+)
> 
> diff --git a/src/box/key_def_api.c b/src/box/key_def_api.c
> index 19590095d..25dd416aa 100644
> --- a/src/box/key_def_api.c
> +++ b/src/box/key_def_api.c
> @@ -205,6 +205,70 @@ box_key_def_delete(box_key_def_t *key_def)
>  	key_def_delete(key_def);
>  }
>  
> +box_key_part_def_t *
> +box_key_def_dump_parts(const box_key_def_t *key_def, uint32_t *part_count_ptr)
> +{
> +	struct region *region = &fiber()->gc;
> +	size_t region_svp = region_used(region);
> +	size_t size;
> +	box_key_part_def_t *parts = region_alloc_array(
> +		region, typeof(parts[0]), key_def->part_count, &size);
> +	if (parts == NULL) {
> +		diag_set(OutOfMemory, size, "region_alloc_array", "parts");
> +		return NULL;
> +	}
> +
> +	for (uint32_t i = 0; i < key_def->part_count; i++) {
> +		const struct key_part *part = &key_def->parts[i];
> +		box_key_part_def_t *part_def = &parts[i];
> +		box_key_part_def_create(part_def);
> +
> +		/* Set part->{fieldno,flags,field_type}. */
> +		part_def->fieldno = part->fieldno;
> +		part_def->flags = 0;
> +		if (key_part_is_nullable(part))
> +			part_def->flags |= BOX_KEY_PART_DEF_IS_NULLABLE_MASK;
> +		assert(part->type >= 0 && part->type < field_type_MAX);
> +		part_def->field_type = field_type_strs[part->type];
> +
> +		/* Set part->collation. */
> +		if (part->coll_id != COLL_NONE) {
> +			struct coll_id *coll_id = coll_by_id(part->coll_id);
> +			/*
> +			 * A collation may be removed after
> +			 * key_def creation.
> +			 */
> +			if (coll_id == NULL) {
> +				diag_set(CollationError,
> +					 "key_def holds dead collation id %d",
> +					 part->coll_id);
> +				region_truncate(region, region_svp);
> +				return NULL;
> +			}
> +			part_def->collation = coll_id->name;

3. What if coll_id is removed after return from this function? For
example, by deleting a tuple from _collation. The pointer would
be dead. We need to copy the name.

> +		}
> +
> +		/* Set part->path. */
> +		if (part->path != NULL) {
> +			char *path = region_alloc(region, part->path_len + 1);
> +			if (path == NULL) {
> +				diag_set(OutOfMemory, part->path_len + 1,
> +					 "region", "part_def->path");
> +				region_truncate(region, region_svp);
> +				return NULL;
> +			}
> +			memcpy(path, part->path, part->path_len);
> +			path[part->path_len] = '\0';
> +			part_def->path = path;
> +		}
> +	}
> +
> +	if (part_count_ptr != NULL)
> +		*part_count_ptr = key_def->part_count;
> +
> +	return parts;
> +}