From: Roman Khabibov <roman.habibov@tarantool.org> To: Timur Safin <tsafin@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules Date: Sat, 18 Apr 2020 05:46:06 +0300 [thread overview] Message-ID: <2D88A96E-5B9A-418C-8DD1-BB9BFD1EDD57@tarantool.org> (raw) In-Reply-To: <1e3201d614ae$8cc16d40$a64447c0$@tarantool.org> > On Apr 17, 2020, at 14:51, Timur Safin <tsafin@tarantool.org> wrote: > > > > : -----Original Message----- > : From: Roman Khabibov <roman.habibov@tarantool.org> > : Sent: Friday, April 17, 2020 2:25 PM > : To: Timur Safin <tsafin@tarantool.org> > : Cc: Mergen Imeev <imeevma@tarantool.org>; 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 <tsafin@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@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@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()
next prev parent reply other threads:[~2020-04-18 2:46 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-13 4:11 Roman Khabibov 2020-04-15 19:41 ` Mergen Imeev 2020-04-16 0:34 ` Roman Khabibov 2020-04-16 8:17 ` Mergen Imeev 2020-04-17 0:48 ` Roman Khabibov 2020-04-17 6:35 ` Mergen Imeev 2020-04-17 7:27 ` Timur Safin 2020-04-17 11:25 ` Roman Khabibov 2020-04-17 11:51 ` Timur Safin 2020-04-18 2:46 ` Roman Khabibov [this message] 2020-04-18 9:18 ` Timur Safin 2020-04-20 0:12 ` Roman Khabibov 2020-04-20 1:05 ` Nikita Pettik 2020-04-20 20:28 ` Roman Khabibov 2020-04-20 23:57 ` Nikita Pettik 2020-04-24 13:03 ` Roman Khabibov 2020-04-27 1:34 ` Nikita Pettik 2020-04-27 15:31 ` Nikita Pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2D88A96E-5B9A-418C-8DD1-BB9BFD1EDD57@tarantool.org \ --to=roman.habibov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tsafin@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox