From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 0DAF04696C3 for ; Mon, 20 Apr 2020 23:28:34 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <20200420010518.GA25021@tarantool.org> Date: Mon, 20 Apr 2020 23:28:32 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1207B8C8-659E-44EB-80F1-9ABBF6B65FA1@tarantool.org> References: <43EE02FF-1F00-4FBE-AC41-C8C58B390003@tarantool.org> <20200416081731.GA16144@tarantool.org> <6CC4ABB2-D49D-4721-B913-B999D70844FB@tarantool.org> <20200417063512.GA10928@tarantool.org> <1c1301d61489$a1af1790$e50d46b0$@tarantool.org> <8A7E5353-3471-4B54-91F7-F35AB4A4B039@tarantool.org> <1e3201d614ae$8cc16d40$a64447c0$@tarantool.org> <2D88A96E-5B9A-418C-8DD1-BB9BFD1EDD57@tarantool.org> <203801d61562$5ce44390$16accab0$@tarantool.org> <610CF1D6-A724-4BE2-A717-C3F44A733D7A@tarantool.org> <20200420010518.GA25021@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Hi! Yes. > On Apr 20, 2020, at 04:05, Nikita Pettik = wrote: >=20 > On 20 Apr 03:12, Roman Khabibov wrote: >> Hi! Kirill, two LGTMs. >=20 > Don't hurry, I'd like to look at patch as well. Could you please > re-send patch after all fixes? >=20 >>> On Apr 18, 2020, at 12:18, Timur Safin wrote: >>>=20 >>> If you would be asking me then LGTM now >>>=20 >>> Timur >>>=20 >>> : -----Original Message----- >>> : From: Roman Khabibov >>> : Sent: Saturday, April 18, 2020 5:46 AM >>> : To: Timur Safin >>> : Cc: Mergen Imeev ; tarantool- >>> : patches@dev.tarantool.org >>> : Subject: Re: [PATCH] sql: fix number and boolean sorting rules >>> :=20 >>> :=20 >>> : > On Apr 17, 2020, at 14:51, Timur Safin = wrote: >>> : > >>> : > >>> : > >>> : > : -----Original Message----- >>> : > : From: Roman Khabibov >>> : > : Sent: Friday, April 17, 2020 2:25 PM >>> : > : To: Timur Safin >>> : > : Cc: Mergen Imeev ; tarantool- >>> : > : patches@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 = 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 >>> : > : Date: Mon Apr 13 05:03:54 2020 +0300 >>> : > : >>> : > : sql: fix sorting rules for scalar >>> : > : >>> : > : Sort values=E2=80=8Bin 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 =3D 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=E2=80=99t understand that, sorry. Now it is done. >>> :=20 >>> : commit 48530ef30fbbb3c7983e75385adbc43448fb9732 (HEAD -> = romanhabibov/gh- >>> : 4697-scalar-bug) >>> : Author: Roman Khabibov >>> : Date: Mon Apr 13 05:03:54 2020 +0300 >>> :=20 >>> : sql: fix sorting rules for scalar >>> :=20 >>> : Sort values =E2=80=8Bin the correct order for scalar type when = using the >>> : vdbe sorter. Boolean always precedes number if it is sorted in >>> : ascending order. >>> :=20 >>> : Closes #4697 >>> :=20 >>> : 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 =3D double_compare_uint64(pKey2->u.r, >>> : mem1.u.u, = -1); >>> : } else { >>> : - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; >>> : + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || >>> : + (pKey2->flags & MEM_Bool) !=3D 0 ? = 1 : -1; >>> : } >>> : break; >>> : } >>> : @@ -3072,7 +3073,8 @@ sqlVdbeCompareMsgpack(const char **key1, >>> : rc =3D = double_compare_uint64(-pKey2->u.r, >>> : -mem1.u.i, = 1); >>> : } else { >>> : - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; >>> : + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || >>> : + (pKey2->flags & MEM_Bool) !=3D 0 ? = 1 : -1; >>> : } >>> : break; >>> : } >>> : @@ -3096,7 +3098,8 @@ sqlVdbeCompareMsgpack(const char **key1, >>> : rc =3D +1; >>> : } >>> : } else { >>> : - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; >>> : + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || >>> : + (pKey2->flags & MEM_Bool) !=3D 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 =3D 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; >>> : + ]], { >>> : + -- >>> : + true,"boolean",1,"integer",1.1,"double" >>> : + }) >>> : + >>> : +test:do_execsql_test( >>> : + "scalar-bug-2.0", >>> : + [[ >>> : + SELECT s3, typeof(s3) FROM test ORDER BY s3; >>> : + ]], { >>> : + -- >>> : + true,"boolean",1,"integer",1.1,"double" >>> : + }) >>> : + >>> : +test:finish_test() >>>=20 >>>=20 >>=20 commit 48530ef30fbbb3c7983e75385adbc43448fb9732 (HEAD -> = romanhabibov/gh-4697-scalar-bug, origin/romanhabibov/gh-4697-scalar-bug) Author: Roman Khabibov Date: Mon Apr 13 05:03:54 2020 +0300 sql: fix sorting rules for scalar =20 Sort values =E2=80=8Bin the correct order for scalar type when using = the vdbe sorter. Boolean always precedes number if it is sorted in ascending order. =20 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 =3D double_compare_uint64(pKey2->u.r, mem1.u.u, = -1); } else { - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || + (pKey2->flags & MEM_Bool) !=3D 0 ? = 1 : -1; } break; } @@ -3072,7 +3073,8 @@ sqlVdbeCompareMsgpack(const char **key1, rc =3D = double_compare_uint64(-pKey2->u.r, -mem1.u.i, = 1); } else { - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || + (pKey2->flags & MEM_Bool) !=3D 0 ? = 1 : -1; } break; } @@ -3096,7 +3098,8 @@ sqlVdbeCompareMsgpack(const char **key1, rc =3D +1; } } else { - rc =3D (pKey2->flags & MEM_Null) ? +1 : = -1; + rc =3D (pKey2->flags & MEM_Null) !=3D 0 = || + (pKey2->flags & MEM_Bool) !=3D 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 =3D 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; + ]], { + -- + true,"boolean",1,"integer",1.1,"double" + }) + +test:do_execsql_test( + "scalar-bug-2.0", + [[ + SELECT s3, typeof(s3) FROM test ORDER BY s3; + ]], { + -- + true,"boolean",1,"integer",1.1,"double" + }) + +test:finish_test()