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 A86114696C3 for ; Mon, 20 Apr 2020 04:05:19 +0300 (MSK) Date: Mon, 20 Apr 2020 01:05:18 +0000 From: Nikita Pettik Message-ID: <20200420010518.GA25021@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> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <610CF1D6-A724-4BE2-A717-C3F44A733D7A@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: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org 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 wrote: > > > > 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 > > : > > : > > : > 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​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/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 > > : 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; > > : + ]], { > > : + -- > > : + 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() > > > > >