Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: v.shpilevoy@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v2 13/15] sql: remove copying of result in finalizers
Date: Tue, 21 Sep 2021 13:59:31 +0300	[thread overview]
Message-ID: <a9ea2350922b160beabb993d12d6e426f81db7b6.1632220375.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1632220375.git.imeevma@gmail.com>

Thank you for the review. I replaced struct sql_context by struct MEM as the
only argument of finalizer functions.

Diff:

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c524cd949..cdfb020cb 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -84,12 +84,13 @@ step_total(struct sql_context *ctx, int argc, struct Mem **argv)
 }
 
 /** Finalizer for the TOTAL() function. */
-static void
-fin_total(struct sql_context *ctx)
+static int
+fin_total(struct Mem *mem)
 {
-	assert(ctx->pOut->type == MEM_TYPE_NULL || mem_is_double(ctx->pOut));
-	if (ctx->pOut->type == MEM_TYPE_NULL)
-		mem_set_double(ctx->pOut, 0.0);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_double(mem));
+	if (mem->type == MEM_TYPE_NULL)
+		mem_set_double(mem, 0.0);
+	return 0;
 }
 
 /** Implementation of the AVG() function. */
@@ -125,19 +126,20 @@ 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(ctx->pOut->type == MEM_TYPE_NULL || mem_is_bin(ctx->pOut));
-	if (ctx->pOut->type == MEM_TYPE_NULL)
-		return mem_set_null(ctx->pOut);
-	struct Mem *mem = (struct Mem *)ctx->pOut->z;
-	uint32_t *count = (uint32_t *)(mem + 1);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_bin(mem));
+	if (mem->type == MEM_TYPE_NULL) {
+		mem_set_null(mem);
+		return 0;
+	}
+	struct Mem *sum = (struct Mem *)mem->z;
+	uint32_t *count = (uint32_t *)(sum + 1);
 	struct Mem mem_count;
 	mem_create(&mem_count);
 	mem_set_uint(&mem_count, *count);
-	if (mem_div(mem, &mem_count, ctx->pOut) != 0)
-		ctx->is_aborted = true;
+	return mem_div(sum, &mem_count, mem);
 }
 
 /** Implementation of the COUNT() function. */
@@ -154,12 +156,13 @@ step_count(struct sql_context *ctx, int argc, struct Mem **argv)
 }
 
 /** Finalizer for the COUNT() function. */
-static void
-fin_count(struct sql_context *ctx)
+static int
+fin_count(struct Mem *mem)
 {
-	assert(ctx->pOut->type == MEM_TYPE_NULL || mem_is_uint(ctx->pOut));
-	if (ctx->pOut->type == MEM_TYPE_NULL)
-		return mem_set_uint(ctx->pOut, 0);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_uint(mem));
+	if (mem->type == MEM_TYPE_NULL)
+		mem_set_uint(mem, 0);
+	return 0;
 }
 
 /** Implementation of the MIN() and MAX() functions. */
@@ -1908,7 +1911,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);
 };
 
 /**
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 8782c072a..97e71e82c 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4371,7 +4371,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 1d22c4a40..65bb60d2f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4201,11 +4201,7 @@ case OP_AggFinal: {
 	struct Mem *pIn1 = &aMem[pOp->p1];
 
 	if (func->finalize != NULL) {
-		struct sql_context ctx;
-		memset(&ctx, 0, sizeof(ctx));
-		ctx.pOut = pIn1;
-		func->finalize(&ctx);
-		if (ctx.is_aborted)
+		if (func->finalize(pIn1) != 0)
 			goto abort_due_to_error;
 	}
 	UPDATE_MAX_BLOBSIZE(pIn1);


New patch:


commit a9ea2350922b160beabb993d12d6e426f81db7b6
Author: Mergen Imeev <imeevma@gmail.com>
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 001a8641c..cdfb020cb 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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_num(ctx->pMem));
+	assert(ctx->pOut->type == MEM_TYPE_NULL || mem_is_num(ctx->pOut));
 	if (argv[0]->type == MEM_TYPE_NULL)
 		return;
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		return mem_copy_as_ephemeral(ctx->pMem, argv[0]);
-	if (mem_add(ctx->pMem, argv[0], ctx->pMem) != 0)
+	if (ctx->pOut->type == MEM_TYPE_NULL)
+		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(ctx->pMem->type == MEM_TYPE_NULL || 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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_num(ctx->pMem));
+	assert(ctx->pOut->type == MEM_TYPE_NULL || mem_is_num(ctx->pOut));
 	if (argv[0]->type == MEM_TYPE_NULL)
 		return;
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		mem_set_double(ctx->pMem, 0.0);
-	if (mem_add(ctx->pMem, argv[0], ctx->pMem) != 0)
+	if (ctx->pOut->type == MEM_TYPE_NULL)
+		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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_double(ctx->pMem));
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		mem_set_double(ctx->pOut, 0.0);
-	else
-		mem_copy_as_ephemeral(ctx->pOut, ctx->pMem);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_double(mem));
+	if (mem->type == MEM_TYPE_NULL)
+		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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_bin(ctx->pMem));
+	assert(ctx->pOut->type == MEM_TYPE_NULL || mem_is_bin(ctx->pOut));
 	if (argv[0]->type == MEM_TYPE_NULL)
 		return;
 	struct Mem *mem;
 	uint32_t *count;
-	if (ctx->pMem->type == MEM_TYPE_NULL) {
+	if (ctx->pOut->type == MEM_TYPE_NULL) {
 		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,19 +126,20 @@ 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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_bin(ctx->pMem));
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		return mem_set_null(ctx->pOut);
-	struct Mem *mem = (struct Mem *)ctx->pMem->z;
-	uint32_t *count = (uint32_t *)(mem + 1);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_bin(mem));
+	if (mem->type == MEM_TYPE_NULL) {
+		mem_set_null(mem);
+		return 0;
+	}
+	struct Mem *sum = (struct Mem *)mem->z;
+	uint32_t *count = (uint32_t *)(sum + 1);
 	struct Mem mem_count;
 	mem_create(&mem_count);
 	mem_set_uint(&mem_count, *count);
-	if (mem_div(mem, &mem_count, ctx->pOut) != 0)
-		ctx->is_aborted = true;
+	return mem_div(sum, &mem_count, mem);
 }
 
 /** Implementation of the COUNT() function. */
@@ -155,22 +147,22 @@ static void
 step_count(struct sql_context *ctx, int argc, struct Mem **argv)
 {
 	assert(argc == 0 || argc == 1);
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		mem_set_uint(ctx->pMem, 0);
+	if (ctx->pOut->type == MEM_TYPE_NULL)
+		mem_set_uint(ctx->pOut, 0);
 	if (argc == 1 && argv[0]->type == MEM_TYPE_NULL)
 		return;
-	assert(ctx->pMem->type == MEM_TYPE_UINT);
-	++ctx->pMem->u.u;
+	assert(ctx->pOut->type == MEM_TYPE_UINT);
+	++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(ctx->pMem->type == MEM_TYPE_NULL || mem_is_uint(ctx->pMem));
-	if (ctx->pMem->type == MEM_TYPE_NULL)
-		return mem_set_uint(ctx->pOut, 0);
-	mem_copy_as_ephemeral(ctx->pOut, ctx->pMem);
+	assert(mem->type == MEM_TYPE_NULL || mem_is_uint(mem));
+	if (mem->type == MEM_TYPE_NULL)
+		mem_set_uint(mem, 0);
+	return 0;
 }
 
 /** Implementation of the MIN() and MAX() functions. */
@@ -180,12 +172,12 @@ step_minmax(struct sql_context *ctx, int argc, struct Mem **argv)
 	assert(argc == 1);
 	(void)argc;
 	if (argv[0]->type == MEM_TYPE_NULL) {
-		if (ctx->pMem->type != MEM_TYPE_NULL)
+		if (ctx->pOut->type != MEM_TYPE_NULL)
 			ctx->skipFlag = 1;
 		return;
 	}
-	if (ctx->pMem->type == MEM_TYPE_NULL) {
-		if (mem_copy(ctx->pMem, argv[0]) != 0)
+	if (ctx->pOut->type == MEM_TYPE_NULL) {
+		if (mem_copy(ctx->pOut, argv[0]) != 0)
 			ctx->is_aborted = true;
 		return;
 	}
@@ -197,22 +189,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)
@@ -222,8 +207,8 @@ step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv)
 	if (argv[0]->type == MEM_TYPE_NULL)
 		return;
 	assert(mem_is_str(argv[0]) || mem_is_bin(argv[0]));
-	if (ctx->pMem->type == MEM_TYPE_NULL) {
-		if (mem_copy_str(ctx->pMem, argv[0]->z, argv[0]->n) != 0)
+	if (ctx->pOut->type == MEM_TYPE_NULL) {
+		if (mem_copy_str(ctx->pOut, argv[0]->z, argv[0]->n) != 0)
 			ctx->is_aborted = true;
 		return;
 	}
@@ -241,24 +226,17 @@ step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv)
 		sep_len = argv[1]->n;
 	}
 	if (sep_len > 0) {
-		if (mem_append(ctx->pMem, sep, sep_len) != 0) {
+		if (mem_append(ctx->pOut, sep, sep_len) != 0) {
 			ctx->is_aborted = true;
 			return;
 		}
 	}
-	if (mem_append(ctx->pMem, argv[0]->z, argv[0]->n) != 0) {
+	if (mem_append(ctx->pOut, argv[0]->z, argv[0]->n) != 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)
 {
@@ -1933,7 +1911,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);
 };
 
 /**
@@ -1970,13 +1948,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,
@@ -2007,35 +1985,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},
@@ -2069,9 +2035,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,
@@ -2330,7 +2296,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 710bd86cf..97e71e82c 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);
 
@@ -4383,7 +4371,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 8804e3d18..65bb60d2f 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,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)
+			goto abort_due_to_error;
+	}
 	UPDATE_MAX_BLOBSIZE(pIn1);
 	if (sqlVdbeMemTooBig(pIn1) != 0)
 		goto too_big;
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.
  */

  parent reply	other threads:[~2021-09-21 11:05 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1632220375.git.imeevma@gmail.com>
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 01/15] sql: fix possible undefined behavior during cast Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 02/15] sql: use register P1 for number of arguments Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 04/15] sql: move collation to struct sql_context Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 05/15] sql: introduce mem_append() Mergen Imeev via Tarantool-patches
2021-09-25 11:06   ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 06/15] sql: remove sql_vdbemem_finalize() Mergen Imeev via Tarantool-patches
2021-09-22 22:47   ` Vladislav Shpilevoy via Tarantool-patches
2021-09-25 11:13     ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 07/15] sql: rework SUM() Mergen Imeev via Tarantool-patches
2021-09-22 22:48   ` Vladislav Shpilevoy via Tarantool-patches
2021-09-25 11:17     ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 08/15] sql: rework TOTAL() Mergen Imeev via Tarantool-patches
2021-09-25 11:20   ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 09/15] sql: rework AVG() Mergen Imeev via Tarantool-patches
2021-09-22 22:48   ` Vladislav Shpilevoy via Tarantool-patches
2021-09-25 11:32     ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 10/15] sql: rework COUNT() Mergen Imeev via Tarantool-patches
2021-09-25 11:34   ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 11/15] sql: rework MIN() and MAX() Mergen Imeev via Tarantool-patches
2021-09-25 11:36   ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 12/15] sql: rework GROUP_CONCAT() Mergen Imeev via Tarantool-patches
2021-09-22 22:49   ` Vladislav Shpilevoy via Tarantool-patches
2021-09-25 11:42     ` Mergen Imeev via Tarantool-patches
2021-09-29  7:03       ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` Mergen Imeev via Tarantool-patches [this message]
2021-09-22 22:50   ` [Tarantool-patches] [PATCH v2 13/15] sql: remove copying of result in finalizers Vladislav Shpilevoy via Tarantool-patches
2021-09-25 11:47     ` Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 14/15] sql: remove MEM_TYPE_AGG Mergen Imeev via Tarantool-patches
2021-09-21 10:59 ` [Tarantool-patches] [PATCH v2 15/15] sql: remove field argv from struct sql_context Mergen Imeev via Tarantool-patches
2021-09-22 22:51   ` Vladislav Shpilevoy via Tarantool-patches
2021-09-25 12:03     ` Mergen Imeev via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9ea2350922b160beabb993d12d6e426f81db7b6.1632220375.git.imeevma@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 13/15] sql: remove copying of result in finalizers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox