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 383DC6EC40; Mon, 16 Aug 2021 16:53:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 383DC6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629122000; bh=e4yUNwJZmjFXB6jIPWVTm5FeEaumUjDa0ZGcKsPvfSE=; 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=MGrSxN5JzhY3mnaU5Xpkwk7mf/F/jvmsDxTYwbaAtScsGYn7K027lNjvY+c53NgHq Cv7MQ7GG7rTPVrNeuwyD2ASFltYrU1rV1wOt6MWNz/lyL/hDtrDgP+ftNuyZwepYiZ RA1wVlgzipY0tQsEpSSCd39COueB41IWNdvurSVI= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (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 4779A6EC40 for ; Mon, 16 Aug 2021 16:53:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4779A6EC40 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1mFd3G-0002bg-IR; Mon, 16 Aug 2021 16:53:18 +0300 Date: Mon, 16 Aug 2021 16:53:17 +0300 To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210816135317.mxk7oi2xytgu5lnt@esperanza> References: <7f889561e6e2fe205d91224eaf6c5c3e439a2a41.1628824421.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7f889561e6e2fe205d91224eaf6c5c3e439a2a41.1628824421.git.imeevma@gmail.com> X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9F9A2272A1D086A28553D1D5C4B4124EF182A05F5380850407C3E1E54C9C904DEF609458CCE1506CCD04221C4DE36BDDF693499F9DA0BC36E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE76AB1B6FB25ACEDC9EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B6B039AA0E19584A8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D5F96099BEF9E2DAE4360D04FA44383D117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751F618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A56DBBA2708F043694B97A5FCF61E3A6EF636EB6AFF78C6A1CD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752DA3D96DA0CEF5C48E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D041FB2E16F174C44A79244FAB96AB018879AD8353DFCFAA1871DBE1E052FF8F0A17E869FD894C731D7E09C32AA3244C64848367C8566024AF4A225A8307D810A95CA90A1D8AC565FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojIrFL/N5KnVERxliOK9KqmQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D24B56C5FE7A8200FF30EBB75F422B90D274CEFED1673C562683ABF942079399BFB559BB5D741EB966A65DFF43FF7BE03240331F90058701C67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 02/10] sql: rework SQL built-in functions hash table 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" On Fri, Aug 13, 2021 at 06:17:07AM +0300, imeevma@tarantool.org wrote: > @@ -2615,23 +2099,30 @@ built_in_func_put(struct func *func) > } > } > > +static struct func * > +find_built_in_func(struct Expr *expr, struct sql_func_dictionary *dict) > +{ > + const char *name = expr->u.zToken; > + int n = expr->x.pList != NULL ? expr->x.pList->nExpr : 0; > + struct func *func = &dict->functions[0]->base; > + assert(func->def->exports.sql); > + int param_count = func->def->param_count; > + if (param_count != -1 && param_count != n) { > + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name, > + tt_sprintf("%d", func->def->param_count), n); > + return NULL; > + } > + return func; > +} > + > struct func * > sql_func_find(struct Expr *expr) > { > const char *name = expr->u.zToken; > - int n = expr->x.pList ? expr->x.pList->nExpr : 0; > - struct func *func = built_in_func_get(name); > - if (func != NULL) { > - assert(func->def->exports.sql); > - int param_count = func->def->param_count; > - if (param_count != -1 && param_count != n) { > - diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name, > - tt_sprintf("%d", func->def->param_count), n); > - return NULL; > - } > - return func; > - } > - func = func_by_name(name, strlen(name)); > + struct sql_func_dictionary *dict = built_in_func_get(name); > + if (dict != NULL) > + return find_built_in_func(expr, dict); > + struct func *func = func_by_name(name, strlen(name)); > if (func == NULL) { > diag_set(ClientError, ER_NO_SUCH_FUNCTION, name); > return NULL; Having a separate (from _func) two-level (name -> dictionary -> func) hash for built-in functions looks ugly. Treating built-in functions and user-defined functions differently here, but using the same struct func for both kinds looks ugly as well. The old code, where both built-in functions and user-defined functions lived in the same hash (and in _func system space), looked consistent and neat. If our goal is to enable static type checking of function arguments, we can instead add a callback to struct func ('check'), which would raise an error if supplied arguments are incorrect without invoking the function. We wouldn't have a separate 'execute' callback for each variant of the same function then, because all variants would share the same struct func, like they do now, but do we really need it? Another idea is to move built-in function name resolution completely to the parser. Then we wouldn't need a hash for built-in functions at all (nor would we need to have them in _func). I find this idea particularly appealing, because after all built-in functions are like operators - their names and argument types are known at the parse time - so theoretically we could generate a separate opcode for them with all the necessary information prepared by the paser. Please check out if this is doable.