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 F17E72465C for ; Thu, 12 Sep 2019 08:13:58 -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 4G3aKlknDGrG for ; Thu, 12 Sep 2019 08:13:58 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 A757822C85 for ; Thu, 12 Sep 2019 08:13:58 -0400 (EDT) Date: Thu, 12 Sep 2019 15:13:55 +0300 From: Nikita Pettik Subject: [tarantool-patches] Re: [PATCH v2 3/3] sql: use name instead of function pointer for UDF Message-ID: <20190912121352.GB92833@tarantool.org> References: <68b5bf44a11ea134d4a3a52f2086aa9ea640e729.1568275504.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <68b5bf44a11ea134d4a3a52f2086aa9ea640e729.1568275504.git.kshcherbatov@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: tarantool-patches@freelists.org Cc: Kirill Shcherbatov On 12 Sep 11:06, Kirill Shcherbatov wrote: > This patch changes OP_Function parameters convention: now a > function's name is passed instead of pointer to the function > object. This allows to normally handle the situation, when UDF > had been deleted to the moment of the VDBE code execution. Nit: has been deleted. > In particular case this may happen with CK constraints that > refer to a persistent function. -> refer to a deleted persistent function (?). > --- > src/box/sql/expr.c | 17 ++++++++++++----- > src/box/sql/vdbe.c | 17 +++++++++++------ > src/box/sql/vdbe.h | 1 + > test/sql/checks.result | 10 +++++++--- > test/sql/checks.test.lua | 3 ++- > 5 files changed, 33 insertions(+), 15 deletions(-) > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 6d5ad2a27..04c9393be 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -4136,11 +4136,18 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > sqlVdbeAddOp4(v, OP_CollSeq, 0, 0, 0, > (char *)coll, P4_COLLSEQ); > } > - int op = func->def->language == > - FUNC_LANGUAGE_SQL_BUILTIN ? > - OP_BuiltinFunction0 : OP_Function; > - sqlVdbeAddOp4(v, op, constMask, r1, target, > - (char *)func, P4_FUNC); > + if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) { > + sqlVdbeAddOp4(v, OP_BuiltinFunction0, constMask, > + r1, target, (char *)func, > + P4_FUNC); > + } else { > + sqlVdbeAddOp4(v, OP_Function, > + func->def->name_len, r1, target, Why not null-terminate function's name? Then you don't have to deal with name's length. > + sqlDbStrNDup(sql_get(), Nit: you can use pParse->db. > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index 02e16da19..4c6245348 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -1795,16 +1795,21 @@ case OP_BuiltinFunction: { > break; > } > > -/* Opcode: Function * P2 P3 P4 P5 > +/* Opcode: Function P1 P2 P3 P4 P5 > * Synopsis: r[P3]=func(r[P2@P5]) > * > - * Invoke a user function (P4 is a pointer to a function object > - * that defines the function) with P5 arguments taken from > - * register P2 and successors. The result of the function is > - * stored in register P3. > + * Invoke a user function (P4 is a name of the function while P1 > + * is a function name length) with P5 arguments taken from register P2 and Nit: out of 72 :) > + * successors. The result of the function is stored in > + * register P3. > */ > case OP_Function: { > - struct func *func = pOp->p4.func; > + assert(pOp->p4type == P4_DYNAMIC); > + struct func *func = func_by_name(pOp->p4.z, pOp->p1); > + if (unlikely(func == NULL)) { > + diag_set(ClientError, ER_NO_SUCH_FUNCTION, pOp->p4.z); > + goto abort_due_to_error; > + } > int argc = pOp->p5; > struct Mem *argv = &aMem[pOp->p2]; > struct port args, ret; > diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h > index 29ff99867..e57d7f979 100644 > --- a/src/box/sql/vdbe.h > +++ b/src/box/sql/vdbe.h > @@ -128,6 +128,7 @@ struct SubProgram { > #define P4_COLLSEQ (-3) /* P4 is a pointer to a CollSeq structure */ > /** P4 is a pointer to a func structure. */ > #define P4_FUNC (-4) > +/** P4 is a function identifier. */ Nit: redundant diff.