[Tarantool-patches] [PATCH 14/14] WIP: module api: add box_key_def_validate_key()

[Alexander Turenko]

> > +int
> > +box_key_def_validate_key(const box_key_def_t *key_def, const char *key,
> > +			 bool allow_nullable)
> 
> I don't like exposing such internal parameters as 'allow_nullable'. It
> took me some time to find why is it even needed. Maybe better export 2
> separate functions?
> 
>     box_key_def_validate_exact_key
>     box_key_def_validate_range_key
> 
> Or
> 
>     box_key_def_validate_unique_key
>     box_key_def_validate_key
> 
> Or
> 
>     box_key_def_validate_unique_key
>     box_key_def_validate_non_unique_key
> 
> Or
> 
>     box_key_def_validate_primary_key
>     box_key_def_validate_secondary_key
> 
> Also did you think about making this function not setting any error?
> What if a module code does not want to spoil diag, and just wants
> to check the key? So as to set an own error later. Maybe would be
> more flexible to expose a simple 'is_valid' instead of 'validate'?

To be honest, I don't like 'allow_nullable' name too. I had guess that
it would be more intuitive than 'is_exact', but it seems, it is not.

In fact, the flag is like a hack to validate a key against a key_def as
if all parts would be defined as non-nullable. Nothing prevents a user
to create the same key_def, but non-nullable (_dump_parts(), change
nullability of parts, call _new_v2()).

So I will just remove the flag. If we'll really need it, we surely will
able to add a function that will be named using words 'exact' and 'key'.

Aside of this, please, look at the discussion around names of tuple and
key_def related functions in [1]. It relates to this function as well.

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/019989.html

> 
> Or do we do that to get a more precise error message and this is
> more important than not setting a diag when not needed?

I would say 'yes'. The diagnostic area is a kind of errno and I don't
see anything bad in filling it even when not any caller will read the
error.