Re: [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions

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

Thanks for the patch!

On 14.08.2020 17:05, imeevma@tarantool.org wrote:
> This patch adds overload function definitions for the length(),
> substr(), trim(), and position() functions.

You should describe how the overload works. How name collisions are
resolved.

Also I want to say I am against these 'overloads' for string/varbinary.
The code looks ugly with the new argument 'has_blob_arg' and with changing
the function name. Also it adds **2** new flags to think about, when you
work with the functions.

It should be either more generic with proper function names mangling like
in C++ (this is also bad), or it should not exist and the types should be
resolved inside single function taking both string and blob. Currently the
'overloaded' VARBINARY functions even return exactly the same types as their
non-VARBINARY versions (what is probably a bug - SUBSTR on blob returns
string?) anyway.

Or it should be separated into different functions defined explicitly.
So as user would be aware he needs to use LENGTH for strings, LENGTH_BIN
for byte length of blobs, etc. Otherwise you are trying to invent some
kind of implicit casts, but you cast functions instead of types. Which is
not better.