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 130036EC40; Thu, 19 Aug 2021 08:32:12 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 130036EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1629351132; bh=61EyQwG4mgVVLUeLM3WtYqX4se4ESN76trxxIJpH5Qs=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=J3d/WgTOsx+Mkterc52glMnPuPJW3PqPU0ao/2eFxrxD71l2tcS2IeQnXkSpmyhTp VG/WwM6cjVeDXLZsuLnra12Zixu5ks6aY28RHVtII6WngAYNxaqPGmniAsg+XPuJfe zkdAiSHl1dKZj4kpAHsXAkc51KGFa2BbdR+I3HSs= 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 12BEA6EC43 for ; Thu, 19 Aug 2021 08:31:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 12BEA6EC43 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1mGaeE-000062-Sd; Thu, 19 Aug 2021 08:31:27 +0300 To: vdavydov@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Thu, 19 Aug 2021 08:31:26 +0300 Message-Id: X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD906AB4890CDABF0C5CB76CEE71D3E4007182A05F538085040F122CFA20AC569CC45280214591E7B2C0C5205A39151D8F803381A919034B6AB X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79B5CC362CEDE941CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B3FBAE160100E6228638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C88027F60915C9E2BDF0C0EC58C50EB8117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B974A882099E279BDA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CE31A2885C41F97C443847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CF160826E4E1956AE62E073B9A28BBB664694FC8349A6A6DC9C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF5A3EDA775A1E0ED0699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3458239C6F30CE7ACFF6A7E71F998BDB281C7D5D1E587AB0BCA69D61E9C88C3B321237F9B3346E81831D7E09C32AA3244C61E569A54B425E50E2F30B3CDB33B8F3D9ADFF0C0BDB8D1F729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojGSxK+6r6oBG3wDgN+rkYVA== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D56C455D5A04B0AC97BD6A5A7CC808A2D83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v2 1/5] sql: remove OP_BuiltinFunction0 and OP_AggStep0 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: Mergen Imeev via Tarantool-patches Reply-To: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This patch moves the initialization of sql_context out of the VDBE. This allows us to remove the opcodes OP_BuiltinFunction0 and OP_AggStep0, which work in a rather strange way. Moreover, due to the changes these opcodes make to the VDBEs, it is possible that the estimated size of the VDBE could become non-constant, which could lead to various problems. Part of #6105 --- src/box/sql/analyze.c | 6 +-- src/box/sql/expr.c | 2 +- src/box/sql/func.c | 13 ++++++- src/box/sql/select.c | 2 +- src/box/sql/vdbe.c | 87 ++----------------------------------------- src/box/sql/vdbeInt.h | 2 +- src/box/sql/vdbeaux.c | 2 +- 7 files changed, 22 insertions(+), 92 deletions(-) diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index fb4ad2a81..b9b6f5da8 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -725,7 +725,7 @@ callStatGet(Vdbe * v, int regStat4, int iParam, int regOut) */ struct func *func = sql_func_by_signature("_sql_stat_get", 2); assert(func != NULL); - sqlVdbeAddOp4(v, OP_BuiltinFunction0, 0, regStat4, regOut, + sqlVdbeAddOp4(v, OP_BuiltinFunction, 0, regStat4, regOut, (char *)func, P4_FUNC); sqlVdbeChangeP5(v, 2); } @@ -869,7 +869,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space) struct func *init_func = sql_func_by_signature("_sql_stat_init", 3); assert(init_func != NULL); - sqlVdbeAddOp4(v, OP_BuiltinFunction0, 0, stat4_reg + 1, + sqlVdbeAddOp4(v, OP_BuiltinFunction, 0, stat4_reg + 1, stat4_reg, (char *)init_func, P4_FUNC); sqlVdbeChangeP5(v, 3); /* @@ -974,7 +974,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space) struct func *push_func = sql_func_by_signature("_sql_stat_push", 3); assert(push_func != NULL); - sqlVdbeAddOp4(v, OP_BuiltinFunction0, 1, stat4_reg, tmp_reg, + sqlVdbeAddOp4(v, OP_BuiltinFunction, 1, stat4_reg, tmp_reg, (char *)push_func, P4_FUNC); sqlVdbeChangeP5(v, 3); sqlVdbeAddOp2(v, OP_Next, idx_cursor, next_row_addr); diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 50dc98a94..d0bdee4fd 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4168,7 +4168,7 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) sqlVdbeAddOp4(v, OP_CollSeq, 0, 0, 0, (char *)coll, P4_COLLSEQ); } - if (sql_emit_func_call(v, pExpr, OP_BuiltinFunction0, + if (sql_emit_func_call(v, pExpr, OP_BuiltinFunction, constMask, r1, target, nFarg) != 0) { pParse->is_aborted = true; diff --git a/src/box/sql/func.c b/src/box/sql/func.c index ea3b44472..4a0d2d097 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -2889,7 +2889,18 @@ sql_emit_func_call(struct Vdbe *vdbe, struct Expr *expr, int op, int mask, struct func *func = sql_func_find(expr); if (func == NULL) return -1; - sqlVdbeAddOp4(vdbe, op, mask, r1, r2, (char *)func, P4_FUNC); + uint32_t size = sizeof(struct sql_context); + if (argc > 1) + size += (argc - 1) * sizeof(struct Mem); + struct sql_context *ctx = sqlDbMallocRawNN(sql_get(), size); + if (ctx == NULL) + return -1; + ctx->pOut = NULL; + ctx->func = func; + ctx->iOp = 0; + ctx->pVdbe = vdbe; + ctx->argc = argc; + sqlVdbeAddOp4(vdbe, op, mask, r1, r2, (char *)ctx, P4_FUNCCTX); sqlVdbeChangeP5(vdbe, argc); return 0; } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 87f2012f1..cb92f2ed0 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -5632,7 +5632,7 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo) sqlVdbeAddOp4(v, OP_CollSeq, regHit, 0, 0, (char *)coll, P4_COLLSEQ); } - sql_emit_func_call(v, pF->pExpr, OP_AggStep0, 0, regAgg, + sql_emit_func_call(v, pF->pExpr, OP_AggStep, 0, regAgg, pF->iMem, nArg); sql_expr_type_cache_change(pParse, regAgg, nArg); sqlReleaseTempRange(pParse, regAgg, nArg); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7f86fa7b3..98ea37c67 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1181,20 +1181,6 @@ case OP_CollSeq: { break; } -/* Opcode: BuiltinFunction0 P1 P2 P3 P4 P5 - * Synopsis: r[P3]=func(r[P2@P5]) - * - * Invoke a user function (P4 is a pointer to a FuncDef object that - * defines the function) with P5 arguments taken from register P2 and - * successors. The result of the function is stored in register P3. - * Register P3 must not be one of the function inputs. - * - * P1 is a 32-bit bitmask indicating whether or not each argument to the - * function was determined to be constant at compile time. If the first - * argument was constant then bit 0 of P1 is set. - * - * See also: BuiltinFunction, AggStep, AggFinal - */ /* Opcode: BuiltinFunction P1 P2 P3 P4 P5 * Synopsis: r[P3]=func(r[P2@P5]) * @@ -1207,44 +1193,15 @@ case OP_CollSeq: { * function was determined to be constant at compile time. If the first * argument was constant then bit 0 of P1 is set. * - * SQL functions are initially coded as OP_BuiltinFunction0 with - * P4 pointing to a FuncDef object. But on first evaluation, - * the P4 operand is automatically converted into an sql_context - * object and the operation changed to this OP_BuiltinFunction - * opcode. In this way, the initialization of the sql_context - * object occurs only once, rather than once for each evaluation - * of the function. - * - * See also: BuiltinFunction0, AggStep, AggFinal + * See also: AggStep, AggFinal */ -case OP_BuiltinFunction0: { - int n; - sql_context *pCtx; - - assert(pOp->p4type == P4_FUNC); - n = pOp->p5; - assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor)); - assert(n==0 || (pOp->p2>0 && pOp->p2+n<=(p->nMem+1 - p->nCursor)+1)); - assert(pOp->p3p2 || pOp->p3>=pOp->p2+n); - pCtx = sqlDbMallocRawNN(db, sizeof(*pCtx) + (n-1)*sizeof(sql_value*)); - if (pCtx==0) goto no_mem; - pCtx->pOut = 0; - pCtx->func = pOp->p4.func; - pCtx->iOp = (int)(pOp - aOp); - pCtx->pVdbe = p; - pCtx->argc = n; - pOp->p4type = P4_FUNCCTX; - pOp->p4.pCtx = pCtx; - pOp->opcode = OP_BuiltinFunction; - /* Fall through into OP_BuiltinFunction */ - FALLTHROUGH; -} case OP_BuiltinFunction: { int i; sql_context *pCtx; assert(pOp->p4type==P4_FUNCCTX); pCtx = pOp->p4.pCtx; + pCtx->iOp = (int)(pOp - aOp); /* If this function is inside of a trigger, the register array in aMem[] * might change from one evaluation to the next. The next block of code @@ -4184,17 +4141,6 @@ case OP_DecrJumpZero: { /* jump, in1 */ } -/* Opcode: AggStep0 * P2 P3 P4 P5 - * Synopsis: accum=r[P3] step(r[P2@P5]) - * - * Execute the step function for an aggregate. The - * function has P5 arguments. P4 is a pointer to the FuncDef - * structure that specifies the function. Register P3 is the - * accumulator. - * - * The P5 arguments are taken from register P2 and its - * successors. - */ /* Opcode: AggStep * P2 P3 P4 P5 * Synopsis: accum=r[P3] step(r[P2@P5]) * @@ -4205,35 +4151,7 @@ case OP_DecrJumpZero: { /* jump, in1 */ * * The P5 arguments are taken from register P2 and its * successors. - * - * This opcode is initially coded as OP_AggStep0. On first evaluation, - * the FuncDef stored in P4 is converted into an sql_context and - * the opcode is changed. In this way, the initialization of the - * sql_context only happens once, instead of on each call to the - * step function. */ -case OP_AggStep0: { - int n; - sql_context *pCtx; - - assert(pOp->p4type == P4_FUNC); - n = pOp->p5; - assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor)); - assert(n==0 || (pOp->p2>0 && pOp->p2+n<=(p->nMem+1 - p->nCursor)+1)); - assert(pOp->p3p2 || pOp->p3>=pOp->p2+n); - pCtx = sqlDbMallocRawNN(db, sizeof(*pCtx) + (n-1)*sizeof(sql_value*)); - if (pCtx==0) goto no_mem; - pCtx->pMem = 0; - pCtx->func = pOp->p4.func; - pCtx->iOp = (int)(pOp - aOp); - pCtx->pVdbe = p; - pCtx->argc = n; - pOp->p4type = P4_FUNCCTX; - pOp->p4.pCtx = pCtx; - pOp->opcode = OP_AggStep; - /* Fall through into OP_AggStep */ - FALLTHROUGH; -} case OP_AggStep: { int i; sql_context *pCtx; @@ -4242,6 +4160,7 @@ case OP_AggStep: { assert(pOp->p4type==P4_FUNCCTX); pCtx = pOp->p4.pCtx; + pCtx->iOp = (int)(pOp - aOp); pMem = &aMem[pOp->p3]; /* If this function is inside of a trigger, the register array in aMem[] diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index cfe743b94..073ac0b97 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -174,7 +174,7 @@ struct sql_context { struct func *func; Mem *pMem; /* Memory cell used to store aggregate context */ Vdbe *pVdbe; /* The VM that owns this context */ - /** Instruction number of OP_BuiltinFunction0. */ + /** Instruction number of OP_BuiltinFunction or OP_AggStep. */ int iOp; /* * True, if an error occurred during the execution of the diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 2d7800b17..662fbbf81 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -1070,7 +1070,7 @@ displayP4(Op * pOp, char *zTemp, int nTemp) } #if defined(SQL_DEBUG) || defined(VDBE_PROFILE) case P4_FUNCCTX:{ - struct func *func = pOp->p4.func; + struct func *func = pOp->p4.pCtx->func; sqlXPrintf(&x, "%s(%d)", func->def->name, func->def->param_count); break; -- 2.25.1