After this patch, all SQL built-in functions will work correctly with DECIMAL values. Closes #6355 --- https://github.com/tarantool/tarantool/issues/6355 https://github.com/tarantool/tarantool/tree/imeevma/gh-6355-introduce-decimal-to-builtins .../gh-6355-introduce-decimal-to-builtins.md | 3 + src/box/sql/func.c | 26 ++++++ src/box/sql/mem.h | 6 ++ test/sql-tap/decimal.test.lua | 81 ++++++++++++++++++- 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md diff --git a/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md b/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md new file mode 100644 index 000000000..2a08f52a4 --- /dev/null +++ b/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md @@ -0,0 +1,3 @@ +## feature/sql + +* Built-in SQL functions now work correctly with DECIMAL values (gh-6355). diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5abaf490d..684fcc275 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -258,6 +258,20 @@ func_abs_double(struct sql_context *ctx, int argc, const struct Mem *argv) mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r); } +static void +func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv) +{ + assert(argc == 1); + (void)argc; + const struct Mem *arg = &argv[0]; + if (mem_is_null(arg)) + return; + assert(mem_is_dec(arg)); + decimal_t dec; + decimal_abs(&dec, &arg->u.d); + mem_set_dec(ctx->pOut, &dec); +} + /** Implementation of the CHAR_LENGTH() function. */ static void func_char_length(struct sql_context *ctx, int argc, const struct Mem *argv) @@ -1694,8 +1708,11 @@ static struct sql_func_definition definitions[] = { NULL}, {"ABS", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_abs_double, NULL}, + {"ABS", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, func_abs_dec, + NULL}, {"AVG", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_avg, fin_avg}, {"AVG", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_avg, fin_avg}, + {"AVG", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, step_avg, fin_avg}, {"CHAR", -1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_STRING, func_char, NULL}, {"CHAR_LENGTH", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_INTEGER, func_char_length, NULL}, @@ -1709,6 +1726,8 @@ static struct sql_func_definition definitions[] = { func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_greatest_least, NULL}, + {"GREATEST", -1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, + func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, @@ -1737,6 +1756,8 @@ static struct sql_func_definition definitions[] = { func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_greatest_least, NULL}, + {"LEAST", -1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, + func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, @@ -1765,6 +1786,7 @@ static struct sql_func_definition definitions[] = { {"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_DECIMAL}, FIELD_TYPE_DECIMAL, step_minmax, NULL}, {"MAX", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MAX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, NULL}, @@ -1774,6 +1796,7 @@ static struct sql_func_definition definitions[] = { {"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_DECIMAL}, FIELD_TYPE_DECIMAL, step_minmax, NULL}, {"MIN", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MIN", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, NULL}, @@ -1816,10 +1839,13 @@ static struct sql_func_definition definitions[] = { FIELD_TYPE_VARBINARY, func_substr_octets, NULL}, {"SUM", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_sum, NULL}, {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, NULL}, + {"SUM", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, 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, 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, func_trim_str, NULL}, diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index e3313ff98..102242d6b 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -147,6 +147,12 @@ mem_is_double(const struct Mem *mem) return mem->type == MEM_TYPE_DOUBLE; } +static inline bool +mem_is_dec(const struct Mem *mem) +{ + return mem->type == MEM_TYPE_DEC; +} + static inline bool mem_is_int(const struct Mem *mem) { diff --git a/test/sql-tap/decimal.test.lua b/test/sql-tap/decimal.test.lua index a6d66b75b..cf574ac79 100755 --- a/test/sql-tap/decimal.test.lua +++ b/test/sql-tap/decimal.test.lua @@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR") package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath local test = require("sqltester") -test:plan(109) +test:plan(117) local dec = require("decimal") local dec1 = dec.new("111") @@ -1018,4 +1018,83 @@ test:do_catchsql_test( "the supported range [-9223372036854775808, 18446744073709551615]" }) + +-- gh-6355: Make sure the SQL built-in functions work properly with DECIMAL. +test:execsql([[ + CREATE TABLE t18 (i INT PRIMARY KEY AUTOINCREMENT, d DECIMAL); + INSERT INTO t18(d) VALUES(123), (-0.7), (9999999999999999999999.0); +]]) + +test:do_execsql_test( + "dec-18.1", + [[ + SELECT typeof(ABS(d)), ABS(d) FROM t18; + ]], { + "decimal", dec.new(123), + "decimal", dec.new(0.7), + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.2", + [[ + SELECT typeof(AVG(d)), AVG(d) FROM t18; + ]], { + "decimal", dec.new('3333333333333333333373.7666666666666667') + }) + +test:do_execsql_test( + "dec-18.3", + [[ + SELECT typeof(GREATEST(d, d * 0 + i)), GREATEST(d, d * 0 + i) FROM t18; + ]], { + "decimal", dec.new(123), + "decimal", dec.new(2), + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.4", + [[ + SELECT typeof(LEAST(d, d * 0 + i)), LEAST(d, d * 0 + i) FROM t18; + ]], { + "decimal", dec.new(1), + "decimal", dec.new(-0.7), + "decimal", dec.new(3) + }) + +test:do_execsql_test( + "dec-18.5", + [[ + SELECT typeof(MAX(d)), MAX(d) FROM t18; + ]], { + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.6", + [[ + SELECT typeof(MIN(d)), MIN(d) FROM t18; + ]], { + "decimal", dec.new(-0.7), + }) + +test:do_execsql_test( + "dec-18.7", + [[ + SELECT typeof(SUM(d)), SUM(d) FROM t18; + ]], { + "decimal", dec.new('10000000000000000000121.3') + }) + +test:do_execsql_test( + "dec-18.8", + [[ + SELECT typeof(TOTAL(d)), TOTAL(d) FROM t18; + ]], { + "double", 1e+22 + }) + +test:execsql([[DROP TABLE t18;]]) + test:finish_test() -- 2.25.1
Hi! Thanks for the patch!
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 5abaf490d..684fcc275 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -258,6 +258,20 @@ func_abs_double(struct sql_context *ctx, int argc, const struct Mem *argv)
> mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r);
> }
>
> +static void
> +func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv)
> +{
> + assert(argc == 1);
> + (void)argc;
> + const struct Mem *arg = &argv[0];
> + if (mem_is_null(arg))
> + return;
> + assert(mem_is_dec(arg));
> + decimal_t dec;
> + decimal_abs(&dec, &arg->u.d);
> + mem_set_dec(ctx->pOut, &dec);
You should be able to use the same argument both for in and out
values:
decimal_abs(&arg->u.d, &arg->u.d);
In the 'documentation' of decNumber it is said to be allowed.
See the comment on the function 'decNumberAbs()'.
Hi! Thank you the review! My answer and diff below. On Mon, Nov 15, 2021 at 10:39:38PM +0100, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > > index 5abaf490d..684fcc275 100644 > > --- a/src/box/sql/func.c > > +++ b/src/box/sql/func.c > > @@ -258,6 +258,20 @@ func_abs_double(struct sql_context *ctx, int argc, const struct Mem *argv) > > mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r); > > } > > > > +static void > > +func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv) > > +{ > > + assert(argc == 1); > > + (void)argc; > > + const struct Mem *arg = &argv[0]; > > + if (mem_is_null(arg)) > > + return; > > + assert(mem_is_dec(arg)); > > + decimal_t dec; > > + decimal_abs(&dec, &arg->u.d); > > + mem_set_dec(ctx->pOut, &dec); > > You should be able to use the same argument both for in and out > values: > > decimal_abs(&arg->u.d, &arg->u.d); > > In the 'documentation' of decNumber it is said to be allowed. > See the comment on the function 'decNumberAbs()'. Fixed. Although we still need to copy from arg to pOut. Diff: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 684fcc275..e3ce6c18e 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -267,9 +267,8 @@ func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv) if (mem_is_null(arg)) return; assert(mem_is_dec(arg)); - decimal_t dec; - decimal_abs(&dec, &arg->u.d); - mem_set_dec(ctx->pOut, &dec); + mem_set_dec(ctx->pOut, &arg->u.d); + decimal_abs(&ctx->pOut->u.d, &ctx->pOut->u.d); } /** Implementation of the CHAR_LENGTH() function. */
Hi! Thanks for the fixes! LGTM.
After this patch, all SQL built-in functions will work correctly with DECIMAL values. Closes #6355 --- https://github.com/tarantool/tarantool/issues/6355 https://github.com/tarantool/tarantool/tree/imeevma/gh-6355-introduce-decimal-to-builtins .../gh-6355-introduce-decimal-to-builtins.md | 3 + src/box/sql/func.c | 25 ++++++ src/box/sql/mem.h | 6 ++ test/sql-tap/decimal.test.lua | 81 ++++++++++++++++++- 4 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md diff --git a/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md b/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md new file mode 100644 index 000000000..2a08f52a4 --- /dev/null +++ b/changelogs/unreleased/gh-6355-introduce-decimal-to-builtins.md @@ -0,0 +1,3 @@ +## feature/sql + +* Built-in SQL functions now work correctly with DECIMAL values (gh-6355). diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5abaf490d..e3ce6c18e 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -258,6 +258,19 @@ func_abs_double(struct sql_context *ctx, int argc, const struct Mem *argv) mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r); } +static void +func_abs_dec(struct sql_context *ctx, int argc, const struct Mem *argv) +{ + assert(argc == 1); + (void)argc; + const struct Mem *arg = &argv[0]; + if (mem_is_null(arg)) + return; + assert(mem_is_dec(arg)); + mem_set_dec(ctx->pOut, &arg->u.d); + decimal_abs(&ctx->pOut->u.d, &ctx->pOut->u.d); +} + /** Implementation of the CHAR_LENGTH() function. */ static void func_char_length(struct sql_context *ctx, int argc, const struct Mem *argv) @@ -1694,8 +1707,11 @@ static struct sql_func_definition definitions[] = { NULL}, {"ABS", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_abs_double, NULL}, + {"ABS", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, func_abs_dec, + NULL}, {"AVG", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_avg, fin_avg}, {"AVG", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_avg, fin_avg}, + {"AVG", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, step_avg, fin_avg}, {"CHAR", -1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_STRING, func_char, NULL}, {"CHAR_LENGTH", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_INTEGER, func_char_length, NULL}, @@ -1709,6 +1725,8 @@ static struct sql_func_definition definitions[] = { func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_greatest_least, NULL}, + {"GREATEST", -1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, + func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, func_greatest_least, NULL}, {"GREATEST", -1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, @@ -1737,6 +1755,8 @@ static struct sql_func_definition definitions[] = { func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, func_greatest_least, NULL}, + {"LEAST", -1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, + func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, func_greatest_least, NULL}, {"LEAST", -1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, @@ -1765,6 +1785,7 @@ static struct sql_func_definition definitions[] = { {"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_DECIMAL}, FIELD_TYPE_DECIMAL, step_minmax, NULL}, {"MAX", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MAX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, NULL}, @@ -1774,6 +1795,7 @@ static struct sql_func_definition definitions[] = { {"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_DECIMAL}, FIELD_TYPE_DECIMAL, step_minmax, NULL}, {"MIN", 1, {FIELD_TYPE_NUMBER}, FIELD_TYPE_NUMBER, step_minmax, NULL}, {"MIN", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, step_minmax, NULL}, @@ -1816,10 +1838,13 @@ static struct sql_func_definition definitions[] = { FIELD_TYPE_VARBINARY, func_substr_octets, NULL}, {"SUM", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_sum, NULL}, {"SUM", 1, {FIELD_TYPE_DOUBLE}, FIELD_TYPE_DOUBLE, step_sum, NULL}, + {"SUM", 1, {FIELD_TYPE_DECIMAL}, FIELD_TYPE_DECIMAL, 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, 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, func_trim_str, NULL}, diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 9d5245708..6a04883be 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -147,6 +147,12 @@ mem_is_double(const struct Mem *mem) return mem->type == MEM_TYPE_DOUBLE; } +static inline bool +mem_is_dec(const struct Mem *mem) +{ + return mem->type == MEM_TYPE_DEC; +} + static inline bool mem_is_int(const struct Mem *mem) { diff --git a/test/sql-tap/decimal.test.lua b/test/sql-tap/decimal.test.lua index a6d66b75b..cf574ac79 100755 --- a/test/sql-tap/decimal.test.lua +++ b/test/sql-tap/decimal.test.lua @@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR") package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath local test = require("sqltester") -test:plan(109) +test:plan(117) local dec = require("decimal") local dec1 = dec.new("111") @@ -1018,4 +1018,83 @@ test:do_catchsql_test( "the supported range [-9223372036854775808, 18446744073709551615]" }) + +-- gh-6355: Make sure the SQL built-in functions work properly with DECIMAL. +test:execsql([[ + CREATE TABLE t18 (i INT PRIMARY KEY AUTOINCREMENT, d DECIMAL); + INSERT INTO t18(d) VALUES(123), (-0.7), (9999999999999999999999.0); +]]) + +test:do_execsql_test( + "dec-18.1", + [[ + SELECT typeof(ABS(d)), ABS(d) FROM t18; + ]], { + "decimal", dec.new(123), + "decimal", dec.new(0.7), + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.2", + [[ + SELECT typeof(AVG(d)), AVG(d) FROM t18; + ]], { + "decimal", dec.new('3333333333333333333373.7666666666666667') + }) + +test:do_execsql_test( + "dec-18.3", + [[ + SELECT typeof(GREATEST(d, d * 0 + i)), GREATEST(d, d * 0 + i) FROM t18; + ]], { + "decimal", dec.new(123), + "decimal", dec.new(2), + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.4", + [[ + SELECT typeof(LEAST(d, d * 0 + i)), LEAST(d, d * 0 + i) FROM t18; + ]], { + "decimal", dec.new(1), + "decimal", dec.new(-0.7), + "decimal", dec.new(3) + }) + +test:do_execsql_test( + "dec-18.5", + [[ + SELECT typeof(MAX(d)), MAX(d) FROM t18; + ]], { + "decimal", dec.new('9999999999999999999999') + }) + +test:do_execsql_test( + "dec-18.6", + [[ + SELECT typeof(MIN(d)), MIN(d) FROM t18; + ]], { + "decimal", dec.new(-0.7), + }) + +test:do_execsql_test( + "dec-18.7", + [[ + SELECT typeof(SUM(d)), SUM(d) FROM t18; + ]], { + "decimal", dec.new('10000000000000000000121.3') + }) + +test:do_execsql_test( + "dec-18.8", + [[ + SELECT typeof(TOTAL(d)), TOTAL(d) FROM t18; + ]], { + "double", 1e+22 + }) + +test:execsql([[DROP TABLE t18;]]) + test:finish_test() -- 2.25.1
Hello,
On 22 ноя 10:41, imeevma@tarantool.org wrote:
> After this patch, all SQL built-in functions will work correctly with
> DECIMAL values.
>
> Closes #6355
LGTM. I've checked your patch into master.
--
Regards, Kirill Yukhin