[Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules
Roman Khabibov
roman.habibov at tarantool.org
Sat Apr 18 05:46:06 MSK 2020
> On Apr 17, 2020, at 14:51, Timur Safin <tsafin at tarantool.org> wrote:
>
>
>
> : -----Original Message-----
> : From: Roman Khabibov <roman.habibov at tarantool.org>
> : Sent: Friday, April 17, 2020 2:25 PM
> : To: Timur Safin <tsafin at tarantool.org>
> : Cc: Mergen Imeev <imeevma at tarantool.org>; tarantool-
> : patches at dev.tarantool.org
> : Subject: Re: [PATCH] sql: fix number and boolean sorting rules
> :
> : 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).
> : >
>
>
> : >
> : 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/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.
> : +--
>
> Not everywhere (see the same problem above ^ in the test code)
>
> Timur
>
> P.S.
> That was the purpose of 'g' modifier in my sed expression :)
> To replace all occurrences, not only the first one.
Didn’t understand that, sorry. Now it is done.
commit 48530ef30fbbb3c7983e75385adbc43448fb9732 (HEAD -> 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 values in 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..4ac286f77
--- /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 precedes 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