[Tarantool-patches] [PATCH v2 04/10] box: add new options for functions

[Vladislav Shpilevoy]

Thanks for the patch!

On 14.08.2020 17:04, imeevma at tarantool.org wrote:
> The has_vararg option allows us to work with functions with a variable
> number of arguments. This is required for built-in SQL functions.
> 
> Suppose this option is TRUE for a built-in SQL function. Then:
> 1) If param_list is empty, all arguments can be of any type.
> 2) If the length of param_list is not less than the number of the given
> arguments, the types of the given arguments must be compatible with the
> corresponding types described in param_list.
> 3) If the length of param_list is less than the number of given
> arguments, the rest of the arguments must be compatible with the last
> type in param_list.

How is it going to be used? For example, how will it work with printf(),
where tail of the arguments can be of any type? Do you define it as
scalar then?

> The is_overloaded option allows us to deal with functions that can take
> STRING or VARBINARY arguments. In case the first of these arguments is
> of type VARBINARY, we use the overloaded version of the function. By
> default, we use the STRING version of the function.
> 
> The has_overload option indicates that the function has an overloaded
> version that accepts VARBINARY. See an explanation of the is_overloaded
> option.

In addition to this whole idea with overloads being a huge crutch which also
affects the public API with adding some new intricate options, it also looks
like has_overload is entirely a runtime option. Why is it even stored in
_func?

What if I have a function taking STRING, and I want to add VARBINARY, I
need to update 2 records in _func for that? Looks really inconvenient.

I would try to think more into how not to store these options in _func. Because
you are trying to describe some code from the executable's file using the
storage. From there most of the problems arise - new complicated options for
_func, hard upgrade process, and unnecessary changes of the schema.

Why can't you specify all these flags inside sql_builtins array, and
copy them into func_def from there? It already works fine this way.