[Tarantool-patches] [PATCH v1 1/1] sql: fix comparison between DECIMAL and big DOUBLE
Mergen Imeev
imeevma at tarantool.org
Thu Sep 9 10:39:57 MSK 2021
Hi! Thank you for the review! My answers and new patch below.
On Tue, Sep 07, 2021 at 02:40:25PM +0300, Igor Munkin wrote:
> Mergen,
>
> Thanks for the patch! LGTM, with a few nits regarding the commit
> message.
>
> On 30.08.21, Mergen Imeev via Tarantool-patches wrote:
> > This patch fixes comparison between DECIMAL value and DOUBLE values
> > greater or equal to 1e38 or less or equal to -1e38. Now any DOUBLE value
> > greater or equal to 1e38 is more than any DECIMAL value and DOUBLE
> > value less or equal to -1e38 is less than any DECIMAL value.
>
> Minor: Considering Vlad's review[1], I would explicitly mention the
> order of the operands to be compared (DECIMAL is the left one, DOUBLE is
> the right one). The opposite case works fine even prior to the patch.
>
Thanks, fixed.
> Minor: It would be clearer, if you mention here that there are only 38
> decimal digits in DECIMAL representation (hence, 1e38).
>
Fixed.
> >
> > Closes #6376
> > ---
> > https://github.com/tarantool/tarantool/issues/6376
> > https://github.com/tarantool/tarantool/tree/imeevma/gh-6376-fix-cmp-between-big-double-and-dec
> >
> > .../gh-6376-fix-incorrect-dec-inf-cmp.md | 3 ++
> > src/box/sql/mem.c | 4 +-
> > test/sql-tap/engine.cfg | 1 +
> > .../gh-6376-wrong-double-to-dec-cmp.test.lua | 38 +++++++++++++++++++
> > 4 files changed, 44 insertions(+), 2 deletions(-)
> > create mode 100644 changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md
> > create mode 100755 test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua
> >
>
> <snipped>
>
> > --
> > 2.25.1
> >
>
> [1]: https://lists.tarantool.org/tarantool-patches/003f11f0-5ecc-2e69-dc89-47a34b5f24ac@tarantool.org/
>
> --
> Best regards,
> IM
commit 04ce137be225ea4ee4ef83dd973a099a8cf385f6
Author: Mergen Imeev <imeevma at gmail.com>
Date: Mon Aug 23 09:34:23 2021 +0300
sql: fix comparison between DECIMAL and big DOUBLE
This patch fixes the comparison between DECIMAL as a left value and
DOUBLE greater than or equal to 1e38 or less than or equal to -1e38 as a
right value. Any DOUBLE value greater than or equal to 1e38 is now
greater than any DECIMAL value, and a DOUBLE value less than or equal to
-1e38 is less than any DECIMAL value. This is because our decimal cannot
contain more than 38 digits.
Closes #6376
diff --git a/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md b/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md
new file mode 100644
index 000000000..70de655f1
--- /dev/null
+++ b/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md
@@ -0,0 +1,3 @@
+## bugfix/sql
+
+* Fixed wrong comparison between DECIMAL and large DOUBLE values (gh-6376).
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 48755a017..b7af00723 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -2452,9 +2452,9 @@ mem_cmp_num(const struct Mem *a, const struct Mem *b)
}
case MEM_TYPE_DOUBLE: {
if (b->u.r >= 1e38)
- return 1;
- if (b->u.r <= -1e38)
return -1;
+ if (b->u.r <= -1e38)
+ return 1;
decimal_t dec;
decimal_t *d = decimal_from_double(&dec, b->u.r);
assert(d != NULL && d == &dec);
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index c35d1dced..a6f03307f 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -36,6 +36,7 @@
"memtx": {"engine": "memtx"}
},
"gh-6157-unnecessary-free-on-string.test.lua": {},
+ "gh-6376-wrong-double-to-dec-cmp.test.lua": {},
"gh-4077-iproto-execute-no-bind.test.lua": {},
"gh-6375-assert-on-unsupported-ext.test.lua": {},
"*": {
diff --git a/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua b/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua
new file mode 100755
index 000000000..edfc851a2
--- /dev/null
+++ b/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua
@@ -0,0 +1,38 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(4)
+
+-- Make sure that the comparison between DECIMAL and large DOUBLE is correct.
+test:do_execsql_test(
+ "gh-6376-1",
+ [[
+ SELECT CAST(1 AS DECIMAL) < -1e40;
+ ]], {
+ false
+ })
+
+test:do_execsql_test(
+ "gh-6376-2",
+ [[
+ SELECT CAST(-1 AS DECIMAL) > -1e400;
+ ]], {
+ true
+ })
+
+test:do_execsql_test(
+ "gh-6376-3",
+ [[
+ SELECT CAST(1 AS DECIMAL) <= 1e40;
+ ]], {
+ true
+ })
+
+test:do_execsql_test(
+ "gh-6376-4",
+ [[
+ SELECT CAST(1 AS DECIMAL) >= 1e400;
+ ]], {
+ false
+ })
+
+test:finish_test()
More information about the Tarantool-patches
mailing list