[Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules
Nikita Pettik
korablev at tarantool.org
Mon Apr 20 04:05:18 MSK 2020
On 20 Apr 03:12, Roman Khabibov wrote:
> Hi! Kirill, two LGTMs.
Don't hurry, I'd like to look at patch as well. Could you please
re-send patch after all fixes?
> > On Apr 18, 2020, at 12:18, Timur Safin <tsafin at tarantool.org> wrote:
> >
> > If you would be asking me then LGTM now
> >
> > Timur
> >
> > : -----Original Message-----
> > : From: Roman Khabibov <roman.habibov at tarantool.org>
> > : Sent: Saturday, April 18, 2020 5:46 AM
> > : 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
> > :
> > :
> > : > 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