[Tarantool-patches] [PATCH v2 2/5] sql: remove unnecessary MEM finalization

imeevma at tarantool.org imeevma at tarantool.org
Thu Aug 19 08:31:28 MSK 2021


After removing the setting of the error in the patch "sql: modify
arithmetic aggregate functions", we no longer need to finalize the MEM
that has not been finalized. The logic is this: if the MEM has not been
finalized, then something has happened, and we no longer need to
finalize it. The MEM will be freed like a normal MEM with allocated
memory.

Part of #6105
---
 src/box/sql/mem.c     | 34 ++--------------------------------
 src/box/sql/mem.h     | 19 +------------------
 src/box/sql/vdbe.c    | 14 +++++++++++++-
 src/box/sql/vdbeapi.c | 10 ++--------
 4 files changed, 18 insertions(+), 59 deletions(-)

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 74febd182..01d73968c 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -217,11 +217,7 @@ mem_create(struct Mem *mem)
 static inline void
 mem_clear(struct Mem *mem)
 {
-	if ((mem->type & (MEM_TYPE_AGG | MEM_TYPE_FRAME)) != 0 ||
-	    (mem->flags & MEM_Dyn) != 0) {
-		if (mem->type == MEM_TYPE_AGG)
-			sql_vdbemem_finalize(mem, mem->u.func);
-		assert(mem->type != MEM_TYPE_AGG);
+	if (mem->type == MEM_TYPE_FRAME || (mem->flags & MEM_Dyn) != 0) {
 		if ((mem->flags & MEM_Dyn) != 0) {
 			assert(mem->xDel != SQL_DYNAMIC && mem->xDel != NULL);
 			mem->xDel((void *)mem->z);
@@ -610,7 +606,7 @@ mem_set_frame(struct Mem *mem, struct VdbeFrame *frame)
 }
 
 int
-mem_set_agg(struct Mem *mem, struct func *func, int size)
+mem_set_agg(struct Mem *mem, int size)
 {
 	mem_clear(mem);
 	if (size <= 0)
@@ -621,7 +617,6 @@ mem_set_agg(struct Mem *mem, struct func *func, int size)
 	mem->n = size;
 	mem->type = MEM_TYPE_AGG;
 	assert(mem->flags == 0);
-	mem->u.func = func;
 	return 0;
 }
 
@@ -3045,31 +3040,6 @@ sqlVdbeMemTooBig(Mem * p)
 	return 0;
 }
 
-int
-sql_vdbemem_finalize(struct Mem *mem, struct func *func)
-{
-	assert(func != NULL);
-	assert(func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
-	assert(func->def->aggregate == FUNC_AGGREGATE_GROUP);
-	assert(mem->type == MEM_TYPE_NULL || func == mem->u.func);
-	sql_context ctx;
-	memset(&ctx, 0, sizeof(ctx));
-	Mem t;
-	memset(&t, 0, sizeof(t));
-	t.type = MEM_TYPE_NULL;
-	assert(t.flags == 0);
-	t.db = mem->db;
-	ctx.pOut = &t;
-	ctx.pMem = mem;
-	ctx.func = func;
-	((struct func_sql_builtin *)func)->finalize(&ctx);
-	assert((mem->flags & MEM_Dyn) == 0);
-	if (mem->szMalloc > 0)
-		sqlDbFree(mem->db, mem->zMalloc);
-	memcpy(mem, &t, sizeof(t));
-	return ctx.is_aborted ? -1 : 0;
-}
-
 int
 sqlVdbeRecordCompareMsgpack(const void *key1,
 				struct UnpackedRecord *key2)
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 543944b80..bddac1a67 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -70,11 +70,6 @@ struct Mem {
 		bool b;         /* Boolean value used when MEM_Bool is set in flags */
 		int nZero;	/* Used when bit MEM_Zero is set in flags */
 		void *p;	/* Generic pointer */
-		/**
-		 * A pointer to function implementation.
-		 * Used only when flags==MEM_Agg.
-		 */
-		struct func *func;
 		struct VdbeFrame *pFrame;	/* Used when flags==MEM_Frame */
 		struct tt_uuid uuid;
 		decimal_t d;
@@ -569,7 +564,7 @@ mem_set_frame(struct Mem *mem, struct VdbeFrame *frame);
  * hold the accumulation structure for the aggregate function.
  */
 int
-mem_set_agg(struct Mem *mem, struct func *func, int size);
+mem_set_agg(struct Mem *mem, int size);
 
 /** Clear MEM and set it to special, "cleared", NULL. */
 void
@@ -961,18 +956,6 @@ int sqlVdbeMemTooBig(Mem *);
 #define VdbeMemDynamic(X) (((X)->flags & MEM_Dyn) != 0 ||\
 			   ((X)->type & (MEM_TYPE_AGG | MEM_TYPE_FRAME)) != 0)
 
-/** MEM manipulate functions. */
-
-/**
- * Memory cell mem contains the context of an aggregate function.
- * This routine calls the finalize method for that function. The
- * result of the aggregate is stored back into mem.
- *
- * Returns -1 if the finalizer reports an error. 0 otherwise.
- */
-int
-sql_vdbemem_finalize(struct Mem *mem, struct func *func);
-
 /**
  * Perform comparison of two tuples: unpacked (key1) and packed (key2)
  *
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 98ea37c67..9fb103e82 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4219,8 +4219,20 @@ case OP_AggFinal: {
 	assert(pOp->p1>0 && pOp->p1<=(p->nMem+1 - p->nCursor));
 	pMem = &aMem[pOp->p1];
 	assert(mem_is_null(pMem) || mem_is_agg(pMem));
-	if (sql_vdbemem_finalize(pMem, pOp->p4.func) != 0)
+	struct sql_context ctx;
+	memset(&ctx, 0, sizeof(ctx));
+	struct Mem t;
+	mem_create(&t);
+	ctx.pOut = &t;
+	ctx.pMem = pMem;
+	((struct func_sql_builtin *)pOp->p4.func)->finalize(&ctx);
+	if (ctx.is_aborted)
 		goto abort_due_to_error;
+	assert((pMem->flags & MEM_Dyn) == 0);
+	if (pMem->szMalloc > 0)
+		sqlDbFree(pMem->db, pMem->zMalloc);
+	memcpy(pMem, &t, sizeof(t));
+
 	UPDATE_MAX_BLOBSIZE(pMem);
 	if (sqlVdbeMemTooBig(pMem)) {
 		goto too_big;
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 8031ee0dc..52d008c5f 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -383,10 +383,7 @@ sqlStmtCurrentTime(sql_context * p)
 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)
+	if (!mem_is_agg(p->pMem) && mem_set_agg(p->pMem, nByte) != 0)
 		return NULL;
 	void *accum;
 	if (mem_get_agg(p->pMem, &accum) != 0)
@@ -397,12 +394,9 @@ sql_aggregate_context(sql_context * p, int nByte)
 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)
+		if (mem_set_agg(ctx->pMem, sizeof(*mem)) != 0)
 			return NULL;
 		if (mem_get_agg(ctx->pMem, (void **)&mem) != 0)
 			return NULL;
-- 
2.25.1



More information about the Tarantool-patches mailing list