[tarantool-patches] Re: [PATCH v2 7/8] sql: get rid of FuncDef function hash

[Konstantin Osipov]

* Kirill Shcherbatov <kshcherbatov at tarantool.org> [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