From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 581E14696C3 for ; Sat, 18 Apr 2020 12:18:47 +0300 (MSK) From: "Timur Safin" References: <20200413041105.22576-1-roman.habibov@tarantool.org> <20200415194149.GA454@tarantool.org> <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> In-Reply-To: <2D88A96E-5B9A-418C-8DD1-BB9BFD1EDD57@tarantool.org> Date: Sat, 18 Apr 2020 12:18:45 +0300 Message-ID: <203801d61562$5ce44390$16accab0$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: ru 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: 'Roman Khabibov' Cc: tarantool-patches@dev.tarantool.org If you would be asking me then LGTM now Timur : -----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()