Re: [Tarantool-patches] [PATCH v3 15/16] module api: add box_key_def_validate_key()

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

Thanks for the patch!

See 2 comments below.

On 13.10.2020 01:23, Alexander Turenko wrote:
> The function allows to verify a key against a key definition. It accepts
> a partial key.
> 
> Part of #5273
> ---
>  src/box/key_def.c                | 13 +++++
>  src/box/key_def.h                | 21 ++++++++
>  src/exports.h                    |  1 +
>  test/app-tap/module_api.c        | 88 ++++++++++++++++++++++++++++++++
>  test/app-tap/module_api.test.lua |  2 +-
>  5 files changed, 124 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/key_def.c b/src/box/key_def.c
> index da1c23135..55ac79362 100644
> --- a/src/box/key_def.c
> +++ b/src/box/key_def.c
> @@ -627,6 +627,19 @@ box_key_def_extract_key(box_key_def_t *key_def, box_tuple_t *tuple,
>  	return tuple_extract_key(tuple, key_def, multikey_idx, key_size_ptr);
>  }
>  
> +int
> +box_key_def_validate_key(const box_key_def_t *key_def, const char *key)
> +{
> +	uint32_t part_count = mp_decode_array(&key);
> +	if (part_count > key_def->part_count) {
> +		diag_set(ClientError, ER_KEY_PART_COUNT, key_def->part_count,
> +			 part_count);
> +		return -1;
> +	}
> +	const char *key_end;

1. Don't you want to make it an out parameter? We won't be able to add it
later, in case it will be ever needed.

> +	return key_validate_parts(key_def, key, part_count, true, &key_end);
> +}
> +
>  /* }}} Module API functions */
>
> diff --git a/src/box/key_def.h b/src/box/key_def.h
> index 1b27836a8..440f2fb13 100644
> --- a/src/box/key_def.h
> +++ b/src/box/key_def.h
> @@ -545,6 +545,27 @@ API_EXPORT char *
>  box_key_def_extract_key(box_key_def_t *key_def, box_tuple_t *tuple,
>  			int multikey_idx, uint32_t *key_size_ptr);
>  
> +/**
> + * Check a key against given key definition.
> + *
> + * Verifies key parts against given key_def's field types with
> + * respect to nullability.
> + *
> + * A partial key (with less part than defined in @a key_def) is
> + * verified by given key parts, the omitted tail is not verified
> + * anyhow.
> + *
> + * Note: nil is accepted for nullable fields, but only for them.
> + *
> + * @param key_def  Key definition.
> + * @param key      MessagePack'ed data for matching.
> + *
> + * @retval 0   The key is valid.
> + * @retval -1  The key is invalid.

2. Maybe it is worth saying in all the new public functions, that
if they return -1, it means diag is set, and is available via
box_error_message() and shit.