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 45A7D70358; Wed, 15 Sep 2021 00:21:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 45A7D70358 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1631654505; bh=PQpjYAegaHjFQz+lZLxRu34GgWSp/sqfoWAAVEf7TFE=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=kn4W97X3oyV/MX8EaNsGs863ViRhUnv9Vuo4z3T4/PEmbr3z5e+gFqlmvUPMsdvwr P2Z3TmK7lemd3qPhqncK8zxRZt7A5uybNM4T/NmYYSqFvfmjWjFRfFk3TfZdpJTCT6 oaGUTswDmSxCjAnwqQrsotDtdkyf0wHgYBm5D+E4= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 1CAB870358 for ; Wed, 15 Sep 2021 00:21:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1CAB870358 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mQFs7-0006iV-8c; Wed, 15 Sep 2021 00:21:43 +0300 To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: Message-ID: <32e96999-c3d0-1b98-6ff6-61bd0ee2cee8@tarantool.org> Date: Tue, 14 Sep 2021 23:21:42 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD91AE02D33A9C88A2F7631404A3C5D35349DE4D58F47F062C400894C459B0CD1B99410B142A76ABE4ECB322FF9DCF484A24C7EFB8636A59566B87F5774AF1921ED X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77603ADE015AF816DEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063719899BAB9B61B3948638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D89CF5E336BABEF2A1609879AD2D9BCC94117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC974A882099E279BDA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352026055571C92BF10FF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDCE939D40DBB93CA75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505233A9F1ACC4CB22349116C76F0AA96A7 X-C1DE0DAB: 0D63561A33F958A5903F522A7503F18F4D48DFE5D32063C3E7BA8B9290E89ADED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34E54F8089C01448AA2E5D008F32867A210458FF2DFD1A3D2D6D548445A0D1ED2E484FDC0FB26823C01D7E09C32AA3244CBF974026B9B7F9A2FCB207F4777B53FEA8CE788DE6831205729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj2yA91e2A1g0ae5kyXrdZbQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DA1328A332CB38696493C8731423980983841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thanks for the patch! On 10.09.2021 18:01, imeevma@tarantool.org wrote: > 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. There should no be any problems if VDBE size is estimated right before it is copied or whatever the size is used for. Because the updated VDBE would return the updated size as well. Correct? I must say, the old behaviour was quite inventive. Allowed to create an object at runtime and yet not have ifs on the main opcodes paths. I think the reason was to reduce usage of pointers in the opcodes. The goal we used to have too. I am fine with the change though. > 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 > @@ -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); Why do you allocate + size of mem? I don't see where it is saved here or used. ctx->pMem is left not initialized, pOut is zero. So where is this size of mem used? > + 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; > +}