[Tarantool-patches] [PATCH v3 13/15] sql: remove copying of result in finalizers
imeevma at tarantool.org
imeevma at tarantool.org
Fri Oct 1 10:43:20 MSK 2021
This patch removes copying of the result in the finalizers of the SQL
built-in aggregate functions.
Part of #4145
---
src/box/sql/func.c | 174 +++++++++++++++++-------------------------
src/box/sql/sqlInt.h | 14 +---
src/box/sql/vdbe.c | 28 +------
src/box/sql/vdbeInt.h | 1 -
src/box/sql/vdbeapi.c | 39 ----------
5 files changed, 75 insertions(+), 181 deletions(-)
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 93181e89c..6f6908566 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;
if (mem_is_zerobin(argv[0]))
sqlDbFree(sql_get(), str);
}
-/** 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);
};
/**
@@ -1982,13 +1960,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,
@@ -2019,35 +1997,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},
@@ -2081,8 +2047,8 @@ 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_INTEGER}, FIELD_TYPE_INTEGER, step_sum, fin_sum},
- {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, fin_sum},
+ {"SUM", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_sum, NULL},
+ {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, NULL},
{"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, step_total,
fin_total},
{"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_total,
@@ -2333,7 +2299,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.
*/
--
2.25.1
More information about the Tarantool-patches
mailing list