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 1AF64255C4 for ; Tue, 13 Aug 2019 04:42:49 -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 1Ya9HnsHyDnc for ; Tue, 13 Aug 2019 04:42:49 -0400 (EDT) Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (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 B2A0A255C1 for ; Tue, 13 Aug 2019 04:42:48 -0400 (EDT) Date: Tue, 13 Aug 2019 11:42:46 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH v2 7/8] sql: get rid of FuncDef function hash Message-ID: <20190813084246.GA5284@atlas> References: <29c44d7790720584498ca1763cb28ff98e35c71d.1565275470.git.kshcherbatov@tarantool.org> <20190812221150.GU32337@atlas> <28e7766a-0374-36ab-d848-a533106dedda@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28e7766a-0374-36ab-d848-a533106dedda@tarantool.org> 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: Kirill Shcherbatov Cc: Tarantool MailList , Nikita Pettik * Kirill Shcherbatov [19/08/13 10:30]: > >> + 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). What does this code do? Why do you need an anonymous structure, instead of initializing struct func_sql_builtn object right away? What do you need the binary search for? Pre-ordering the array for the binary search is very fragile. > 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. Can this be coded in Lua, in upgrade.lua? -- Konstantin Osipov, Moscow, Russia