From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BE9AC430409 for ; Sat, 22 Aug 2020 17:29:48 +0300 (MSK) References: <34e3d319f2955e21f9fc1d358b778128cea9ed79.1597417321.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: <81667c8f-41b6-6d75-a781-3b8f34a50466@tarantool.org> Date: Sat, 22 Aug 2020 16:29:47 +0200 MIME-Version: 1.0 In-Reply-To: <34e3d319f2955e21f9fc1d358b778128cea9ed79.1597417321.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.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.