From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CBC69256F0 for ; Tue, 13 Aug 2019 03:29:48 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0BvkrzbkgPoO for ; Tue, 13 Aug 2019 03:29:48 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1E2E1255AB for ; Tue, 13 Aug 2019 03:29:47 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 7/8] sql: get rid of FuncDef function hash References: <29c44d7790720584498ca1763cb28ff98e35c71d.1565275470.git.kshcherbatov@tarantool.org> <20190812221150.GU32337@atlas> From: Kirill Shcherbatov Message-ID: <28e7766a-0374-36ab-d848-a533106dedda@tarantool.org> Date: Tue, 13 Aug 2019 10:29:44 +0300 MIME-Version: 1.0 In-Reply-To: <20190812221150.GU32337@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Konstantin Osipov , Tarantool MailList , Nikita Pettik >> + REG_FUNC("ABS", ARGC_MASK(1), FIELD_TYPE_NUMBER, 0, >> + absFunc, NULL, true), > > > This looks like huge overengineering. > > A bunch of macros, a binary search, can you just open-code > inserting all of these functions into the hash? It would be > perhaps a bit more copy-paste, but it will be easy to maintain. > > Am I missing something? This constructor is called automatically during function recovery by prototype defined in snapshot. Thus all functions are initialized in a one place: in on_replace_dd_func trigger. I like this concept because it is uniform and all implementation-dependent details (like the completion of the function definition initialization) are hidden in the corresponding module (sql/func.c). Technically we may skip a function cache entry creation for SQL_BUILTIN tuple type in on_replace_dd_func trigger, to perform the whole preparements during Tarantool initialization. This require to distinguish SQL_BUILTIN entries and other types in trigger and this is not cool, I believe. Finally, a current approach enforces keeping snapshot will all built in definitions in actual state.