[Tarantool-patches] [PATCH v2 08/15] sql: rework TOTAL()
Mergen Imeev
imeevma at tarantool.org
Sat Sep 25 14:20:23 MSK 2021
Thank you for the review! I replaced manual checks of MEM type by mem_is_*()
functions. Diff and new patch below.
On Tue, Sep 21, 2021 at 01:59:18PM +0300, Mergen Imeev via Tarantool-patches wrote:
> This patch makes TOTAL() accept DOUBLE values by default. Also, after
> this patch TOTAL() will be able to work with DECIMAL values.
>
> Part of #4145
> Part of #6355
> ---
> src/box/sql/func.c | 47 ++++++++++++++++--------
> test/sql-tap/built-in-functions.test.lua | 33 ++++++++++++++++-
> 2 files changed, 63 insertions(+), 17 deletions(-)
>
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index b06955302..12a6a5a2c 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -76,6 +76,32 @@ fin_sum(struct sql_context *ctx)
> 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));
> + 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)
> + ctx->is_aborted = true;
> +}
> +
> +/** Finalizer for the TOTAL() function. */
> +static void
> +fin_total(struct sql_context *ctx)
> +{
> + 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);
> +}
> +
> static const unsigned char *
> mem_as_ustr(struct Mem *mem)
> {
> @@ -1693,17 +1719,6 @@ avgFinalize(sql_context * context)
> context->is_aborted = true;
> }
>
> -static void
> -totalFinalize(sql_context * context)
> -{
> - SumCtx *p;
> - p = sql_aggregate_context(context, 0);
> - if (p == NULL || p->count == 0)
> - mem_set_double(context->pOut, 0.0);
> - else
> - mem_copy_as_ephemeral(context->pOut, &p->mem);
> -}
> -
> /*
> * The following structure keeps track of state information for the
> * count() aggregate function.
> @@ -2128,10 +2143,12 @@ static struct sql_func_definition definitions[] = {
> {"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},
> - {"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, sum_step,
> - totalFinalize},
> - {"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, sum_step,
> - totalFinalize},
> + {"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_total,
> + fin_total},
> + {"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, step_total,
> + fin_total},
> + {"TOTAL", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DOUBLE, step_total,
> + fin_total},
>
> {"TRIM", 2, {FIELD_TYPE_STRING, FIELD_TYPE_INTEGER},
> FIELD_TYPE_STRING, trim_func, NULL},
> diff --git a/test/sql-tap/built-in-functions.test.lua b/test/sql-tap/built-in-functions.test.lua
> index 8daf41c1d..507d06549 100755
> --- a/test/sql-tap/built-in-functions.test.lua
> +++ b/test/sql-tap/built-in-functions.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> local test = require("sqltester")
> -test:plan(55)
> +test:plan(58)
>
> local dec = require('decimal')
>
> @@ -497,7 +497,7 @@ test:do_test(
> local res = {pcall(box.execute, [[SELECT TOTAL(?);]], {'1'})}
> return {tostring(res[3])}
> end, {
> - "Type mismatch: can not convert string('1') to integer"
> + "Type mismatch: can not convert string('1') to double"
> })
>
> --
> @@ -576,4 +576,33 @@ test:do_execsql_test(
> }
> )
>
> +-- Make sure TOTAL() accepts DOUBLE by default.
> +test:do_test(
> + "builtins-4.2.1",
> + function()
> + return box.execute([[SELECT TOTAL(?);]], {1}).metadata
> + end, {
> + {name = "COLUMN_1", type = "double"},
> + })
> +
> +test:do_test(
> + "builtins-4.2.2",
> + function()
> + local res = {pcall(box.execute, [[SELECT TOTAL(?);]], {-1ULL})}
> + return {tostring(res[3])}
> + end, {
> + "Type mismatch: can not convert integer(18446744073709551615) to double"
> + })
> +
> +-- Make sure TOTAL() works with DECIMAL properly.
> +test:do_execsql_test(
> + "builtins-4.2.3",
> + [[
> + SELECT TOTAL(cast(column_2 as DECIMAL)) from (values(1), (123.432));
> + ]],
> + {
> + 124.432
> + }
> +)
> +
> test:finish_test()
> --
> 2.25.1
>
Diff:
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 6cd884221..d0606744c 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -82,10 +82,10 @@ 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));
- if (argv[0]->type == MEM_TYPE_NULL)
+ assert(mem_is_null(ctx->pMem) || mem_is_num(ctx->pMem));
+ if (mem_is_null(argv[0]))
return;
- if (ctx->pMem->type == MEM_TYPE_NULL)
+ if (mem_is_null(ctx->pMem))
mem_set_double(ctx->pMem, 0.0);
if (mem_add(ctx->pMem, argv[0], ctx->pMem) != 0)
ctx->is_aborted = true;
@@ -95,8 +95,8 @@ step_total(struct sql_context *ctx, int argc, struct Mem **argv)
static void
fin_total(struct sql_context *ctx)
{
- assert(ctx->pMem->type == MEM_TYPE_NULL || mem_is_double(ctx->pMem));
- if (ctx->pMem->type == MEM_TYPE_NULL)
+ 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);
New patch:
commit 3d6db454539f77cd9c3d96aacdc8c4ff3a74ef2f
Author: Mergen Imeev <imeevma at gmail.com>
Date: Thu Sep 9 18:02:59 2021 +0300
sql: rework TOTAL()
This patch makes TOTAL() accept DOUBLE values by default. Also, after
this patch TOTAL() will be able to work with DECIMAL values.
Part of #4145
Part of #6355
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 4cc6f378b..d0606744c 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -76,6 +76,32 @@ fin_sum(struct sql_context *ctx)
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));
+ 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)
+ ctx->is_aborted = true;
+}
+
+/** Finalizer for the TOTAL() function. */
+static void
+fin_total(struct sql_context *ctx)
+{
+ 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);
+}
+
static const unsigned char *
mem_as_ustr(struct Mem *mem)
{
@@ -1693,17 +1719,6 @@ avgFinalize(sql_context * context)
context->is_aborted = true;
}
-static void
-totalFinalize(sql_context * context)
-{
- SumCtx *p;
- p = sql_aggregate_context(context, 0);
- if (p == NULL || p->count == 0)
- mem_set_double(context->pOut, 0.0);
- else
- mem_copy_as_ephemeral(context->pOut, &p->mem);
-}
-
/*
* The following structure keeps track of state information for the
* count() aggregate function.
@@ -2128,10 +2143,12 @@ static struct sql_func_definition definitions[] = {
{"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},
- {"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, sum_step,
- totalFinalize},
- {"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, sum_step,
- totalFinalize},
+ {"TOTAL", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_total,
+ fin_total},
+ {"TOTAL", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_DOUBLE, step_total,
+ fin_total},
+ {"TOTAL", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DOUBLE, step_total,
+ fin_total},
{"TRIM", 2, {FIELD_TYPE_STRING, FIELD_TYPE_INTEGER},
FIELD_TYPE_STRING, trim_func, NULL},
diff --git a/test/sql-tap/built-in-functions.test.lua b/test/sql-tap/built-in-functions.test.lua
index 8daf41c1d..507d06549 100755
--- a/test/sql-tap/built-in-functions.test.lua
+++ b/test/sql-tap/built-in-functions.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
local test = require("sqltester")
-test:plan(55)
+test:plan(58)
local dec = require('decimal')
@@ -497,7 +497,7 @@ test:do_test(
local res = {pcall(box.execute, [[SELECT TOTAL(?);]], {'1'})}
return {tostring(res[3])}
end, {
- "Type mismatch: can not convert string('1') to integer"
+ "Type mismatch: can not convert string('1') to double"
})
--
@@ -576,4 +576,33 @@ test:do_execsql_test(
}
)
+-- Make sure TOTAL() accepts DOUBLE by default.
+test:do_test(
+ "builtins-4.2.1",
+ function()
+ return box.execute([[SELECT TOTAL(?);]], {1}).metadata
+ end, {
+ {name = "COLUMN_1", type = "double"},
+ })
+
+test:do_test(
+ "builtins-4.2.2",
+ function()
+ local res = {pcall(box.execute, [[SELECT TOTAL(?);]], {-1ULL})}
+ return {tostring(res[3])}
+ end, {
+ "Type mismatch: can not convert integer(18446744073709551615) to double"
+ })
+
+-- Make sure TOTAL() works with DECIMAL properly.
+test:do_execsql_test(
+ "builtins-4.2.3",
+ [[
+ SELECT TOTAL(cast(column_2 as DECIMAL)) from (values(1), (123.432));
+ ]],
+ {
+ 124.432
+ }
+)
+
test:finish_test()
More information about the Tarantool-patches
mailing list