Re: [Tarantool-patches] [PATCH v1 00/10] Check types of SQL built-in functions arguments

[Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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.