From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 74C406EC40; Thu, 19 Aug 2021 14:49:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 74C406EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629373798; bh=9xb7HMsYZ9IBuVUyRyf2lYzCH3opXkTI4B/1Xb+PsZ4=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Hem6O3jjhCCu2BVpN3OQrvjjW9MvsIJ4pYx0mn1OmxgQB52AVZ3el+NYijjdtjS/8 vPX02xkWkkNWAXWCpJvH88Sp3yp4yOScfUT+sUfsFQxiuLgTa0tNBd16ANViTMIsEc 96JbYtrRr/OglBBAKAF7KOd2CI8xfgP5JBLwptlk= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 5644D6EC40 for ; Thu, 19 Aug 2021 14:49:57 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5644D6EC40 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mGgYW-0007yN-Iw; Thu, 19 Aug 2021 14:49:56 +0300 Date: Thu, 19 Aug 2021 14:49:55 +0300 To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210819114955.lf3nzcbwi3vxpd34@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD906AB4890CDABF0C5CB76CEE71D3E4007182A05F538085040FA68A8590159B34A4E11F5FEC37DFAFD8D1BD0BC9387F8DA55DD31034BD023A2 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75263010198C72082EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063707C4856229E8E7E48638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8138D95DCDC3686D2DBDF63AF95921EF9117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505174D7B233CC048AA07975B416AEE3D23 X-C1DE0DAB: 0D63561A33F958A5807BD8BB43485095CD678FDC54331DF50388D17B0CCB38ABD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA758B25CD4253D1D611410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34F0A5F58274334C95DD174BD3AB009838E3D40499CD9A719A1A00170CF4B86486772BF7485BF5F5CA1D7E09C32AA3244CE36C6B4DF1F8CED1F6B7D83007C5EAA8E8FBBEFAE1C4874CFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojGSxK+6r6oBHBVfU8vxV0+w== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D21A01D28096591AB39EA8A0F6D98EF41274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 00/10] Check types of SQL built-in functions arguments X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladimir Davydov via Tarantool-patches Reply-To: Vladimir Davydov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Since moving the function matching to the parser didn't work out (because this would turn builtin function names, such as 'count', into reserved keywords, forcing the user to quote them), I think we should stick with this approach. LGTM, but I think the function hash is messy: - I don't like the two-level structure of the builtin function hash: name -> sql_func_dictionary with array of func_sql_builtin, one array entry per argument set because all builtin functions share the same implementation and this is probably never going to change: {"MAX", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_UUID}, FIELD_TYPE_UUID, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, minmaxStep, minMaxFinalize}, {"MAX", 1, {FIELD_TYPE_SCALAR}, FIELD_TYPE_SCALAR, minmaxStep, minMaxFinalize}, I think we'd better have a separate hash: name -> builtin func The builtin func struct should contain a list or accepted argumented and the corresponding return value. Or maybe a callback, which would take arguments from an SQL expression list and return the expected return value and argument types. This could be more efficient, because we wouldn't have to scan the list of arguments on resolve. - Builtin funcs shouldn't refer to struct func or func_def, because they don't use any information from those structures (permissions, id, is_sandboxed, etc). We should consider reworking the implementation accordingly in the future.