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 6BA406E462; Fri, 10 Sep 2021 19:02:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6BA406E462 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1631289771; bh=4v2KYSD5ONKZ4b0Bvm6R632vkYjV1xxnpWTtzwSoAw8=; 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=Lmuo6XqKqpFC/38inZG1ngZtog6Nj2Ac5osw8xzmhzB6IxIwVyc2VE2n8ek7e2Ip6 ZmJBZ72nuej3SfHzW3IfoArIHk6EoVALApDXehEI24EHPSeaY8FZ20T6BZ27jVPZBQ bnVbQRIIz6yczH8lvVcPSiInfCwiQ16vUy+1CKuE= 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 00D0A6E462 for ; Fri, 10 Sep 2021 19:01:53 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 00D0A6E462 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1mOiyP-0000xF-6W; Fri, 10 Sep 2021 19:01:53 +0300 To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 10 Sep 2021 19:01:52 +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: 4F1203BC0FB41BD91AE02D33A9C88A2FF3F790D2CFB1565D68082CE688654CB400894C459B0CD1B97800B30582EEB4EFE29E91975A20F80059451B28AE1D565951B62821F85756D2 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7207CDA5E227C0D0BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063738C1CCEC8FAC1CB98638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8A0DFD2412EF034A86E0C1CFD854D7437117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD186FD1C55BDD38FC3FD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B66F6A3E018CF4DC80089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CBF3286B3348795C607538F998C64F2E31A5D6CCBA0ECE35C9C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EFB4CA5BC574AE2910699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34411AA3F52B5778B66EBC9B420DE5A6582D8EDF6C04F6432DD2DD752DE33EFEA010C4E23E66993FB41D7E09C32AA3244C5EBD29F70DC557B39DE52B529C76E848E646F07CC2D4F3D8729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFfmM42hctySVYdT/TitQhg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D26B5F814153570C7783A2064ACEABD8C83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v1 02/13] sql: remove AggStep0 and OP_BuiltinFunction0 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 may change during execution of VDBE, which could lead to various problems. Needed for #4145 --- src/box/sql/expr.c | 12 +++++-- src/box/sql/select.c | 27 +++++++++++++-- src/box/sql/sqlInt.h | 3 ++ src/box/sql/vdbe.c | 81 ++----------------------------------------- src/box/sql/vdbeaux.c | 2 +- 5 files changed, 41 insertions(+), 84 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 3b5df5292..6446ef091 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4107,9 +4107,15 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) (char *)coll, P4_COLLSEQ); } if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) { - sqlVdbeAddOp4(v, OP_BuiltinFunction0, nFarg, - r1, target, (char *)func, - P4_FUNC); + struct sql_context *ctx = + sql_context_new(v, func, nFarg); + if (ctx == NULL) { + pParse->is_aborted = true; + return -1; + } + sqlVdbeAddOp4(v, OP_BuiltinFunction, nFarg, + r1, target, (char *)ctx, + P4_FUNCCTX); } else { sqlVdbeAddOp4(v, OP_FunctionByName, nFarg, r1, target, diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 92e40aef6..e27dcb336 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -5639,8 +5639,13 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo) sqlVdbeAddOp4(v, OP_CollSeq, regHit, 0, 0, (char *)coll, P4_COLLSEQ); } - sqlVdbeAddOp3(v, OP_AggStep0, nArg, regAgg, pF->iMem); - sqlVdbeAppendP4(v, pF->func, P4_FUNC); + struct sql_context *ctx = sql_context_new(v, pF->func, nArg); + if (ctx == NULL) { + pParse->is_aborted = true; + return; + } + sqlVdbeAddOp3(v, OP_AggStep, nArg, regAgg, pF->iMem); + sqlVdbeAppendP4(v, ctx, P4_FUNCCTX); sql_expr_type_cache_change(pParse, regAgg, nArg); sqlReleaseTempRange(pParse, regAgg, nArg); if (addrNext) { @@ -6744,3 +6749,21 @@ sql_expr_extract_select(struct Parse *parser, struct Select *select) parser->parsed_ast.expr = sqlExprDup(parser->db, expr_list->a->pExpr, 0); } + +struct sql_context * +sql_context_new(struct Vdbe *vdbe, struct func *func, uint32_t argc) +{ + 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 NULL; + ctx->pOut = NULL; + ctx->func = func; + ctx->is_aborted = false; + ctx->skipFlag = 0; + ctx->pVdbe = vdbe; + ctx->iOp = 0; + return ctx; +} diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 2e250dc29..c2701dbde 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4141,6 +4141,9 @@ char *sqlStrAccumFinish(StrAccum *); void sqlStrAccumReset(StrAccum *); void sqlSelectDestInit(SelectDest *, int, int, int); +struct sql_context * +sql_context_new(struct Vdbe *vdbe, struct func *func, uint32_t argc); + /* * Create an expression to load @a column from datasource * @a src_idx in @a src_list. diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index a0f1be454..2ff7ce8f4 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1182,16 +1182,6 @@ case OP_CollSeq: { break; } -/* Opcode: BuiltinFunction0 P1 P2 P3 P4 * - * Synopsis: r[P3]=func(r[P2@P1]) - * - * Invoke a user function (P4 is a pointer to a FuncDef object that - * defines the function) with P1 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. - * - * See also: BuiltinFunction, AggStep, AggFinal - */ /* Opcode: BuiltinFunction P1 P2 P3 P4 * * Synopsis: r[P3]=func(r[P2@P1]) * @@ -1200,37 +1190,8 @@ case OP_CollSeq: { * 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. * - * 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->p1; - 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; - pOp->p4type = P4_FUNCCTX; - pOp->p4.pCtx = pCtx; - pOp->opcode = OP_BuiltinFunction; - /* Fall through into OP_BuiltinFunction */ - FALLTHROUGH; -} case OP_BuiltinFunction: { int i; int argc = pOp->p1; @@ -1238,6 +1199,7 @@ case OP_BuiltinFunction: { 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 @@ -4178,17 +4140,6 @@ case OP_DecrJumpZero: { /* jump, in1 */ } -/* Opcode: AggStep0 P1 P2 P3 P4 * - * Synopsis: accum=r[P3] step(r[P2@P1]) - * - * Execute the step function for an aggregate. The - * function has P1 arguments. P4 is a pointer to the FuncDef - * structure that specifies the function. Register P3 is the - * accumulator. - * - * The P1 arguments are taken from register P2 and its - * successors. - */ /* Opcode: AggStep P1 P2 P3 P4 * * Synopsis: accum=r[P3] step(r[P2@P1]) * @@ -4199,34 +4150,7 @@ case OP_DecrJumpZero: { /* jump, in1 */ * * The P1 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; - pOp->p4type = P4_FUNCCTX; - pOp->p4.pCtx = pCtx; - pOp->opcode = OP_AggStep; - /* Fall through into OP_AggStep */ - FALLTHROUGH; -} case OP_AggStep: { int i; int argc = pOp->p1; @@ -4236,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/vdbeaux.c b/src/box/sql/vdbeaux.c index 8148ed8b0..e2dcc72ab 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