[Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules
Roman Khabibov
roman.habibov at tarantool.org
Fri Apr 17 14:25:05 MSK 2020
Hi! Thanks for the review.
> On Apr 17, 2020, at 10:27, Timur Safin <tsafin at tarantool.org> wrote:
>
> As a random bypasser I could not resist and not add my 5 kopecks (see below).
>
> : -----Original Message-----
> : From: Mergen Imeev <imeevma at tarantool.org>
> :
> : Hi! Thank you for the fixes. I think that the your
> : commit-message and comments sounds quite strange, but I am
> : not sure that I am qualified enough to say that for sure.
> : Please, send this patch to NIkita. Other than that, LGTM.
> :
>
>
> : > commit 8a2133d84b88a050181d858d6109abf3635a121b (HEAD ->
> : romanhabibov/gh-4697-scalar-bug, origin/romanhabibov/gh-4697-scalar-bug)
> : > Author: Roman Khabibov <roman.habibov at tarantool.org>
> : > Date: Mon Apr 13 05:03:54 2020 +0300
> : >
> : > sql: fix number and boolean SCALAR sorting rules
> : >
> : > Make comparison SCALAR rules in the sql sorter correct for number
> : > and boolean. Boolean should always follow before any number.
> : >
> : > Closes #4697
> : >
>
> Agreed with Mergen that message is confusing, I'd suggest to do this simple replacement to make it clearer
>
> s/follow before/precede/g
>
> Regards,
> Timur
>
Fixed.
commit 386ba8676c0ebd381d9c03b2ebf80abd986de73b (HEAD -> romanhabibov/gh-4697-scalar-bug, origin/romanhabibov/gh-4697-scalar-bug)
Author: Roman Khabibov <roman.habibov at tarantool.org>
Date: Mon Apr 13 05:03:54 2020 +0300
sql: fix sorting rules for scalar
Sort valuesin the correct order for scalar type when using the
vdbe sorter. Boolean always precedes number if it is sorted in
ascending order.
Closes #4697
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index b88726ea1..985c6ef54 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -3054,7 +3054,8 @@ sqlVdbeCompareMsgpack(const char **key1,
rc = double_compare_uint64(pKey2->u.r,
mem1.u.u, -1);
} else {
- rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+ rc = (pKey2->flags & MEM_Null) != 0 ||
+ (pKey2->flags & MEM_Bool) != 0 ? 1 : -1;
}
break;
}
@@ -3072,7 +3073,8 @@ sqlVdbeCompareMsgpack(const char **key1,
rc = double_compare_uint64(-pKey2->u.r,
-mem1.u.i, 1);
} else {
- rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+ rc = (pKey2->flags & MEM_Null) != 0 ||
+ (pKey2->flags & MEM_Bool) != 0 ? 1 : -1;
}
break;
}
@@ -3096,7 +3098,8 @@ sqlVdbeCompareMsgpack(const char **key1,
rc = +1;
}
} else {
- rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+ rc = (pKey2->flags & MEM_Null) != 0 ||
+ (pKey2->flags & MEM_Bool) != 0 ? 1 : -1;
}
break;
}
diff --git a/test/sql-tap/gh-4697-scalar-bug.test.lua b/test/sql-tap/gh-4697-scalar-bug.test.lua
new file mode 100755
index 000000000..6f600f27b
--- /dev/null
+++ b/test/sql-tap/gh-4697-scalar-bug.test.lua
@@ -0,0 +1,35 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+test:plan(2)
+
+--
+-- gh-4679: Make sure that boolean follow before any number within
+-- scalar. Result with order by indexed (using index) and
+-- non-indexed (using no index) must be the same.
+--
+test:execsql [[
+ CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR UNIQUE, s3 SCALAR);
+ INSERT INTO test VALUES (0, 1, 1);
+ INSERT INTO test VALUES (1, 1.1, 1.1);
+ INSERT INTO test VALUES (2, true, true);
+]]
+
+test:do_execsql_test(
+ "scalar-bug-1.0",
+ [[
+ SELECT s2, typeof(s2) FROM test ORDER BY s2;
+ ]], {
+ -- <scalar-bug-1.0>
+ true,"boolean",1,"integer",1.1,"double"
+ })
+
+test:do_execsql_test(
+ "scalar-bug-2.0",
+ [[
+ SELECT s3, typeof(s3) FROM test ORDER BY s3;
+ ]], {
+ -- <scalar-bug-2.0>
+ true,"boolean",1,"integer",1.1,"double"
+ })
+
+test:finish_test()
More information about the Tarantool-patches
mailing list