[tarantool-patches] Re: [PATCH v2 3/3] sql: use name instead of function pointer for UDF
Nikita Pettik
korablev at tarantool.org
Thu Sep 12 15:13:55 MSK 2019
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 at 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.
More information about the Tarantool-patches
mailing list