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 9DAB96EC40; Sat, 25 Sep 2021 14:47:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9DAB96EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1632570424; bh=rsgc3KPB6BIXus61bymxshE4ORQQhS+t0QFwviJuAX0=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=PZFkJkJ2NDtFHRBCQqsx943fT0qfo4GEu0/rAPrJZL3Esa8vlTm6y98KtjZwO90qN vw88hPAAAjccQuPkvuWKoQf39scNezg4Q7kUQ+vXwnYofdOMVdEsRUwOjIlBmnDEYk /AFTrt1eCuaO1TZX3/DBYq/TIv/cOGNavwjGjAwE= 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 2DBCA6EC40 for ; Sat, 25 Sep 2021 14:47:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2DBCA6EC40 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mU690-0001xx-98; Sat, 25 Sep 2021 14:47:02 +0300 Date: Sat, 25 Sep 2021 14:47:01 +0300 To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210925114701.GJ290467@tarantool.org> References: <5c26f6af-064e-b024-5e63-2808ddb97173@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5c26f6af-064e-b024-5e63-2808ddb97173@tarantool.org> X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD96A58C36AA2E99649DFE41A06634A2C4EB30EB11CC09FFC21182A05F538085040DB2A51E6823FB7E4FBF7A19776992160051A761C3B97D55181535F309D836CBC X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BF5628FE6781D09AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063794BAA5DA89D799D78638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8864113691ED2F7D2B7F1E92E2C046D8F117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B62CFFCC7B69C47339089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A52BFAC66AE0FEC9E4520B6F727B8A862EA69821C87C3604A6D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75BFC02AB3DF06BA5A410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34728AF701C68E45395CE2044BCE1A58015C573298746C906FE25EEB62EAD9A7EE2126E03849C3938E1D7E09C32AA3244CE2B93F0BF064F5C145F5A91B3090F08F3C6EB905E3A8056B729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqHUEAOTxd4iU6K5lfwR9gg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D42C336C1535BC742127CA053EAB2F5D583D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 13/15] sql: remove copying of result in finalizers 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: Mergen Imeev Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answer, diff and new patch below. Also, there were quite a few merge conflicts due to changes in reworked functions, but I didn't include them into diff, sorry. On Thu, Sep 23, 2021 at 12:50:40AM +0200, Vladislav Shpilevoy wrote: > Thanks for the fixes! > > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > index 8804e3d18..65bb60d2f 100644 > > --- a/src/box/sql/vdbe.c > > +++ b/src/box/sql/vdbe.c > > @@ -4207,24 +4200,10 @@ case OP_AggFinal: { > > struct func_sql_builtin *func = (struct func_sql_builtin *)pOp->p4.func; > > struct Mem *pIn1 = &aMem[pOp->p1]; > > > > - struct sql_context ctx; > > - memset(&ctx, 0, sizeof(ctx)); > > - struct Mem t; > > - memset(&t, 0, sizeof(t)); > > - t.type = MEM_TYPE_NULL; > > - assert(t.flags == 0); > > - t.db = pIn1->db; > > - ctx.pOut = &t; > > - ctx.pMem = pIn1; > > - ctx.func = pOp->p4.func; > > - func->finalize(&ctx); > > - assert((pIn1->flags & MEM_Dyn) == 0); > > - if (pIn1->szMalloc > 0) > > - sqlDbFree(pIn1->db, pIn1->zMalloc); > > - memcpy(pIn1, &t, sizeof(t)); > > - > > - if (ctx.is_aborted) > > - goto abort_due_to_error; > > + if (func->finalize != NULL) { > > + if (func->finalize(pIn1) != 0) > > Such places might look simpler and shorter if you would > use &&: > > if (func->finalize != NULL && func->finalize(... > > Up to you. Thanks, fixed. Diff: diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 65bb60d2f..5dd68912c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4200,10 +4200,8 @@ case OP_AggFinal: { struct func_sql_builtin *func = (struct func_sql_builtin *)pOp->p4.func; struct Mem *pIn1 = &aMem[pOp->p1]; - if (func->finalize != NULL) { - if (func->finalize(pIn1) != 0) - goto abort_due_to_error; - } + if (func->finalize != NULL && func->finalize(pIn1) != 0) + goto abort_due_to_error; UPDATE_MAX_BLOBSIZE(pIn1); if (sqlVdbeMemTooBig(pIn1) != 0) goto too_big; New patch: commit b45f3f91d38a3dba4a5bc1187a05690f14b4173d Author: Mergen Imeev Date: Thu Sep 9 18:50:33 2021 +0300 sql: remove copying of result in finalizers This patch removes copying of the result in the finalizers of the SQL built-in aggregate functions. Part of #4145 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 28094e258..769caba94 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -59,47 +59,38 @@ step_sum(struct sql_context *ctx, int argc, struct Mem **argv) { assert(argc == 1); (void)argc; - assert(mem_is_null(ctx->pMem) || mem_is_num(ctx->pMem)); + assert(mem_is_null(ctx->pOut) || mem_is_num(ctx->pOut)); if (mem_is_null(argv[0])) return; - if (mem_is_null(ctx->pMem)) - return mem_copy_as_ephemeral(ctx->pMem, argv[0]); - if (mem_add(ctx->pMem, argv[0], ctx->pMem) != 0) + if (mem_is_null(ctx->pOut)) + return mem_copy_as_ephemeral(ctx->pOut, argv[0]); + if (mem_add(ctx->pOut, argv[0], ctx->pOut) != 0) ctx->is_aborted = true; } -/** Finalizer for the SUM() function. */ -static void -fin_sum(struct sql_context *ctx) -{ - assert(mem_is_null(ctx->pMem) || mem_is_num(ctx->pMem)); - mem_copy_as_ephemeral(ctx->pOut, ctx->pMem); -} - /** Implementation of the TOTAL() function. */ static void step_total(struct sql_context *ctx, int argc, struct Mem **argv) { assert(argc == 1); (void)argc; - assert(mem_is_null(ctx->pMem) || mem_is_num(ctx->pMem)); + assert(mem_is_null(ctx->pOut) || mem_is_num(ctx->pOut)); if (mem_is_null(argv[0])) return; - if (mem_is_null(ctx->pMem)) - mem_set_double(ctx->pMem, 0.0); - if (mem_add(ctx->pMem, argv[0], ctx->pMem) != 0) + if (mem_is_null(ctx->pOut)) + mem_set_double(ctx->pOut, 0.0); + if (mem_add(ctx->pOut, argv[0], ctx->pOut) != 0) ctx->is_aborted = true; } /** Finalizer for the TOTAL() function. */ -static void -fin_total(struct sql_context *ctx) +static int +fin_total(struct Mem *mem) { - assert(mem_is_null(ctx->pMem) || mem_is_double(ctx->pMem)); - if (mem_is_null(ctx->pMem)) - mem_set_double(ctx->pOut, 0.0); - else - mem_copy_as_ephemeral(ctx->pOut, ctx->pMem); + assert(mem_is_null(mem) || mem_is_double(mem)); + if (mem_is_null(mem)) + mem_set_double(mem, 0.0); + return 0; } /** Implementation of the AVG() function. */ @@ -108,12 +99,12 @@ step_avg(struct sql_context *ctx, int argc, struct Mem **argv) { assert(argc == 1); (void)argc; - assert(mem_is_null(ctx->pMem) || mem_is_bin(ctx->pMem)); + assert(mem_is_null(ctx->pOut) || mem_is_bin(ctx->pOut)); if (mem_is_null(argv[0])) return; struct Mem *mem; uint32_t *count; - if (mem_is_null(ctx->pMem)) { + if (mem_is_null(ctx->pOut)) { uint32_t size = sizeof(struct Mem) + sizeof(uint32_t); mem = sqlDbMallocRawNN(sql_get(), size); if (mem == NULL) { @@ -124,10 +115,10 @@ step_avg(struct sql_context *ctx, int argc, struct Mem **argv) mem_create(mem); *count = 1; mem_copy_as_ephemeral(mem, argv[0]); - mem_set_bin_allocated(ctx->pMem, (char *)mem, size); + mem_set_bin_allocated(ctx->pOut, (char *)mem, size); return; } - mem = (struct Mem *)ctx->pMem->z; + mem = (struct Mem *)ctx->pOut->z; count = (uint32_t *)(mem + 1); ++*count; if (mem_add(mem, argv[0], mem) != 0) @@ -135,13 +126,15 @@ step_avg(struct sql_context *ctx, int argc, struct Mem **argv) } /** Finalizer for the AVG() function. */ -static void -fin_avg(struct sql_context *ctx) +static int +fin_avg(struct Mem *mem) { - assert(mem_is_null(ctx->pMem) || mem_is_bin(ctx->pMem)); - if (mem_is_null(ctx->pMem)) - return mem_set_null(ctx->pOut); - struct Mem *tmp = (struct Mem *)ctx->pMem->z; + assert(mem_is_null(mem) || mem_is_bin(mem)); + if (mem_is_null(mem)) { + mem_set_null(mem); + return 0; + } + struct Mem *tmp = (struct Mem *)mem->z; uint32_t *count_val = (uint32_t *)(tmp + 1); struct Mem sum; mem_create(&sum); @@ -150,8 +143,7 @@ fin_avg(struct sql_context *ctx) struct Mem count; mem_create(&count); mem_set_uint(&count, *count_val); - if (mem_div(&sum, &count, ctx->pOut) != 0) - ctx->is_aborted = true; + return mem_div(&sum, &count, mem); } /** Implementation of the COUNT() function. */ @@ -159,22 +151,22 @@ static void step_count(struct sql_context *ctx, int argc, struct Mem **argv) { assert(argc == 0 || argc == 1); - if (mem_is_null(ctx->pMem)) - mem_set_uint(ctx->pMem, 0); + if (mem_is_null(ctx->pOut)) + mem_set_uint(ctx->pOut, 0); if (argc == 1 && mem_is_null(argv[0])) return; - assert(mem_is_uint(ctx->pMem)); - ++ctx->pMem->u.u; + assert(mem_is_uint(ctx->pOut)); + ++ctx->pOut->u.u; } /** Finalizer for the COUNT() function. */ -static void -fin_count(struct sql_context *ctx) +static int +fin_count(struct Mem *mem) { - assert(mem_is_null(ctx->pMem) || mem_is_uint(ctx->pMem)); - if (mem_is_null(ctx->pMem)) - return mem_set_uint(ctx->pOut, 0); - mem_copy_as_ephemeral(ctx->pOut, ctx->pMem); + assert(mem_is_null(mem) || mem_is_uint(mem)); + if (mem_is_null(mem)) + mem_set_uint(mem, 0); + return 0; } /** Implementation of the MIN() and MAX() functions. */ @@ -184,12 +176,12 @@ step_minmax(struct sql_context *ctx, int argc, struct Mem **argv) assert(argc == 1); (void)argc; if (mem_is_null(argv[0])) { - if (!mem_is_null(ctx->pMem)) + if (!mem_is_null(ctx->pOut)) ctx->skipFlag = 1; return; } - if (mem_is_null(ctx->pMem)) { - if (mem_copy(ctx->pMem, argv[0]) != 0) + if (mem_is_null(ctx->pOut)) { + if (mem_copy(ctx->pOut, argv[0]) != 0) ctx->is_aborted = true; return; } @@ -201,22 +193,15 @@ step_minmax(struct sql_context *ctx, int argc, struct Mem **argv) * the only difference between the two being that the sense of the * comparison is inverted. */ - int cmp = mem_cmp_scalar(ctx->pMem, argv[0], ctx->coll); + int cmp = mem_cmp_scalar(ctx->pOut, argv[0], ctx->coll); if ((is_max && cmp < 0) || (!is_max && cmp > 0)) { - if (mem_copy(ctx->pMem, argv[0]) != 0) + if (mem_copy(ctx->pOut, argv[0]) != 0) ctx->is_aborted = true; return; } ctx->skipFlag = 1; } -/** Finalizer for the MIN() and MAX() functions. */ -static void -fin_minmax(struct sql_context *ctx) -{ - mem_copy(ctx->pOut, ctx->pMem); -} - /** Implementation of the GROUP_CONCAT() function. */ static void step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv) @@ -226,12 +211,12 @@ step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv) if (mem_is_null(argv[0])) return; assert(mem_is_str(argv[0]) || mem_is_bin(argv[0])); - if (mem_is_null(ctx->pMem)) { - if (mem_copy(ctx->pMem, argv[0]) != 0) + if (mem_is_null(ctx->pOut)) { + if (mem_copy(ctx->pOut, argv[0]) != 0) ctx->is_aborted = true; return; } - assert(!mem_is_zerobin(ctx->pMem)); + assert(!mem_is_zerobin(ctx->pOut)); const char *sep = NULL; int sep_len = 0; if (argc == 1) { @@ -245,7 +230,7 @@ step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv) sep = argv[1]->z; sep_len = argv[1]->n; } - if (mem_append(ctx->pMem, sep, sep_len) != 0) { + if (mem_append(ctx->pOut, sep, sep_len) != 0) { ctx->is_aborted = true; return; } @@ -259,19 +244,12 @@ step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv) size = argv[0]->n; str = argv[0]->z; } - if (mem_append(ctx->pMem, str, size) != 0) { + if (mem_append(ctx->pOut, str, size) != 0) { ctx->is_aborted = true; return; } } -/** Finalizer for the GROUP_CONCAT() function. */ -static void -fin_group_concat(struct sql_context *ctx) -{ - mem_copy(ctx->pOut, ctx->pMem); -} - static const unsigned char * mem_as_ustr(struct Mem *mem) { @@ -1946,7 +1924,7 @@ struct sql_func_definition { /** Call implementation with given arguments. */ void (*call)(sql_context *ctx, int argc, sql_value **argv); /** Call finalization function for this implementation. */ - void (*finalize)(sql_context *ctx); + int (*finalize)(struct Mem *mem); }; /** @@ -1983,13 +1961,13 @@ static struct sql_func_definition definitions[] = { NULL}, {"GROUP_CONCAT", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, - step_group_concat, fin_group_concat}, + step_group_concat, NULL}, {"GROUP_CONCAT", 2, {FIELD_TYPE_STRING, FIELD_TYPE_STRING}, - FIELD_TYPE_STRING, step_group_concat, fin_group_concat}, + FIELD_TYPE_STRING, step_group_concat, NULL}, {"GROUP_CONCAT", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, - step_group_concat, fin_group_concat}, + step_group_concat, NULL}, {"GROUP_CONCAT", 2, {FIELD_TYPE_VARBINARY, FIELD_TYPE_VARBINARY}, - FIELD_TYPE_VARBINARY, step_group_concat, fin_group_concat}, + FIELD_TYPE_VARBINARY, step_group_concat, NULL}, {"HEX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_STRING, hexFunc, NULL}, {"IFNULL", 2, {FIELD_TYPE_ANY, FIELD_TYPE_ANY}, FIELD_TYPE_SCALAR, @@ -2020,35 +1998,23 @@ static struct sql_func_definition definitions[] = { {"LOWER", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, LowerICUFunc, NULL}, - {"MAX", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_minmax, - fin_minmax}, - {"MAX", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_minmax, - fin_minmax}, - {"MAX", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, - fin_minmax}, + {"MAX", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_minmax, NULL}, + {"MAX", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_minmax, NULL}, + {"MAX", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MAX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, - fin_minmax}, - {"MAX", 1, {FIELD_TYPE_UUID}, FIELD_TYPE_UUID, step_minmax, - fin_minmax}, - {"MAX", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, step_minmax, - fin_minmax}, - {"MAX", 1, {FIELD_TYPE_SCALAR}, FIELD_TYPE_SCALAR, step_minmax, - fin_minmax}, - - {"MIN", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_minmax, - fin_minmax}, - {"MIN", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_minmax, - fin_minmax}, - {"MIN", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, - fin_minmax}, + NULL}, + {"MAX", 1, {FIELD_TYPE_UUID}, FIELD_TYPE_UUID, step_minmax, NULL}, + {"MAX", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, step_minmax, NULL}, + {"MAX", 1, {FIELD_TYPE_SCALAR}, FIELD_TYPE_SCALAR, step_minmax, NULL}, + + {"MIN", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_minmax, NULL}, + {"MIN", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_minmax, NULL}, + {"MIN", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MIN", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, - fin_minmax}, - {"MIN", 1, {FIELD_TYPE_UUID}, FIELD_TYPE_UUID, step_minmax, - fin_minmax}, - {"MIN", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, step_minmax, - fin_minmax}, - {"MIN", 1, {FIELD_TYPE_SCALAR}, FIELD_TYPE_SCALAR, step_minmax, - fin_minmax}, + NULL}, + {"MIN", 1, {FIELD_TYPE_UUID}, FIELD_TYPE_UUID, step_minmax, NULL}, + {"MIN", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, step_minmax, NULL}, + {"MIN", 1, {FIELD_TYPE_SCALAR}, FIELD_TYPE_SCALAR, step_minmax, NULL}, {"NULLIF", 2, {FIELD_TYPE_ANY, FIELD_TYPE_ANY}, FIELD_TYPE_SCALAR, nullifFunc, NULL}, @@ -2082,9 +2048,9 @@ static struct sql_func_definition definitions[] = { {"SUBSTR", 3, {FIELD_TYPE_VARBINARY, FIELD_TYPE_INTEGER, FIELD_TYPE_INTEGER}, FIELD_TYPE_VARBINARY, substrFunc, NULL}, - {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, fin_sum}, - {"SUM", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_sum, fin_sum}, - {"SUM", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, step_sum, fin_sum}, + {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, NULL}, + {"SUM", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_sum, NULL}, + {"SUM", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, step_sum, NULL}, {"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_total, fin_total}, {"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, step_total, @@ -2343,7 +2309,7 @@ sql_built_in_functions_cache_init(void) assert(desc->argc != -1 || dict->argc_min != dict->argc_max); def->param_count = desc->argc; def->returns = desc->result; - def->aggregate = desc->finalize == NULL ? + def->aggregate = (dict->flags & SQL_FUNC_AGG) == 0 ? FUNC_AGGREGATE_NONE : FUNC_AGGREGATE_GROUP; def->language = FUNC_LANGUAGE_SQL_BUILTIN; def->name_len = len; diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index abcc3bfdf..5bce70eaa 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -484,18 +484,6 @@ void sql_row_count(struct sql_context *context, MAYBE_UNUSED int unused1, MAYBE_UNUSED sql_value **unused2); -void * -sql_aggregate_context(sql_context *, - int nBytes); - -/** - * Allocate or return the aggregate context containing struct MEM for a user - * function. A new context is allocated on the first call. Subsequent calls - * return the same context that was returned on prior calls. - */ -struct Mem * -sql_context_agg_mem(struct sql_context *context); - int sql_column_count(sql_stmt * pStmt); @@ -4386,7 +4374,7 @@ struct func_sql_builtin { * A VDBE-memory-compatible finalize method * (is valid only for aggregate function). */ - void (*finalize)(sql_context *ctx); + int (*finalize)(struct Mem *mem); }; /** diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index ee07af9c2..5dd68912c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4145,7 +4145,6 @@ case OP_AggStep: { int argc = pOp->p1; sql_context *pCtx; Mem *pMem; - Mem t; assert(pOp->p4type==P4_FUNCCTX); pCtx = pOp->p4.pCtx; @@ -4156,8 +4155,8 @@ case OP_AggStep: { * checks to see if the register array has changed, and if so it * reinitializes the relavant parts of the sql_context object */ - if (pCtx->pMem != pMem) { - pCtx->pMem = pMem; + if (pCtx->pOut != pMem) { + pCtx->pOut = pMem; for(i = 0; i < argc; ++i) pCtx->argv[i] = &aMem[pOp->p2 + i]; } @@ -4169,18 +4168,12 @@ case OP_AggStep: { } #endif - mem_create(&t); - pCtx->pOut = &t; - pCtx->is_aborted = false; pCtx->skipFlag = 0; assert(pCtx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN); struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func; func->call(pCtx, argc, pCtx->argv); - if (pCtx->is_aborted) { - mem_destroy(&t); + if (pCtx->is_aborted) goto abort_due_to_error; - } - assert(mem_is_null(&t)); if (pCtx->skipFlag) { assert(pOp[-1].opcode == OP_SkipLoad); i = pOp[-1].p1; @@ -4207,20 +4200,7 @@ case OP_AggFinal: { struct func_sql_builtin *func = (struct func_sql_builtin *)pOp->p4.func; struct Mem *pIn1 = &aMem[pOp->p1]; - struct sql_context ctx; - memset(&ctx, 0, sizeof(ctx)); - struct Mem t; - mem_create(&t); - ctx.pOut = &t; - ctx.pMem = pIn1; - ctx.func = pOp->p4.func; - func->finalize(&ctx); - assert((pIn1->flags & MEM_Dyn) == 0); - if (pIn1->szMalloc > 0) - sqlDbFree(pIn1->db, pIn1->zMalloc); - memcpy(pIn1, &t, sizeof(t)); - - if (ctx.is_aborted) + if (func->finalize != NULL && func->finalize(pIn1) != 0) goto abort_due_to_error; UPDATE_MAX_BLOBSIZE(pIn1); if (sqlVdbeMemTooBig(pIn1) != 0) diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index b9a18f1a1..5e2692d06 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -172,7 +172,6 @@ struct sql_context { Mem *pOut; /* The return value is stored here */ /* A pointer to function implementation. */ struct func *func; - Mem *pMem; /* Memory cell used to store aggregate context */ struct coll *coll; /* * True, if an error occurred during the execution of the diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 6e598513c..39f560c32 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -349,45 +349,6 @@ sql_context_db_handle(sql_context * p) return p->pOut->db; } -/* - * Allocate or return the aggregate context for a user function. A new - * context is allocated on the first call. Subsequent calls return the - * same context that was returned on prior calls. - */ -void * -sql_aggregate_context(sql_context * p, int nByte) -{ - assert(p != NULL && p->func != NULL); - assert(p->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN); - assert(p->func->def->aggregate == FUNC_AGGREGATE_GROUP); - if (!mem_is_agg(p->pMem) && mem_set_agg(p->pMem, p->func, nByte) != 0) - return NULL; - void *accum; - if (mem_get_agg(p->pMem, &accum) != 0) - return NULL; - return accum; -} - -struct Mem * -sql_context_agg_mem(struct sql_context *ctx) -{ - assert(ctx != NULL && ctx->func != NULL); - assert(ctx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN); - assert(ctx->func->def->aggregate == FUNC_AGGREGATE_GROUP); - struct Mem *mem; - if (!mem_is_agg(ctx->pMem)) { - if (mem_set_agg(ctx->pMem, ctx->func, sizeof(*mem)) != 0) - return NULL; - if (mem_get_agg(ctx->pMem, (void **)&mem) != 0) - return NULL; - mem_create(mem); - return mem; - } - if (mem_get_agg(ctx->pMem, (void **)&mem) != 0) - return NULL; - return mem; -} - /* * Return the number of columns in the result set for the statement pStmt. */