[Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Make comparison rules in the sql sorter correct for number and
boolean. Boolean should always be greater than any number.

Closes #4697
---
 src/box/sql/vdbeaux.c      |  9 ++++++---
 test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 4 deletions(-)

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/sort.test.lua b/test/sql-tap/sort.test.lua
index 36074d6ef..15b397f1d 100755
--- a/test/sql-tap/sort.test.lua
+++ b/test/sql-tap/sort.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(60)
+test:plan(61)
 
 --!./tcltestrunner.lua
 -- 2001 September 15.
@@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
         })
 
 end
+
+-- gh-4679 Make sure that boolean > any number within scalar.
+test:do_execsql_test(
+    18.1,
+    [[
+        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
+        INSERT INTO test VALUES (0, 1);
+        INSERT INTO test VALUES (2, 1.0);
+        INSERT INTO test VALUES (3, true);
+        SELECT s2, typeof(s2) FROM test ORDER BY s2;
+    ]], {
+        -- <18.1>
+        true,"boolean",1,"integer",1,"double"
+        -- </18.1>
+    })
+
 test:finish_test()
-- 
2.21.0 (Apple Git-122)

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Mergen Imeev]

Hi! Thank you for the patch! See 5 small comments below.

On Mon, Apr 13, 2020 at 07:11:05AM +0300, Roman Khabibov wrote:
> Make comparison rules in the sql sorter correct for number and
> boolean. Boolean should always be greater than any number.
1. I think you should say that values compared here using
SCALAR rules. It makes no sence otherwise, since boolean
cannot be compared with number.

2. Isn't boolean always less that any number?

> 
> Closes #4697
> ---
3. Please include links to issue and branch. Also,
ChangeLog is missing.

>  src/box/sql/vdbeaux.c      |  9 ++++++---
>  test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> 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/sort.test.lua b/test/sql-tap/sort.test.lua
> index 36074d6ef..15b397f1d 100755
> --- a/test/sql-tap/sort.test.lua
> +++ b/test/sql-tap/sort.test.lua
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env tarantool
>  test = require("sqltester")
> -test:plan(60)
> +test:plan(61)
>  
>  --!./tcltestrunner.lua
>  -- 2001 September 15.
> @@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
>          })
>  
>  end
> +
> +-- gh-4679 Make sure that boolean > any number within scalar.
> +test:do_execsql_test(
> +    18.1,
> +    [[
> +        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
> +        INSERT INTO test VALUES (0, 1);
> +        INSERT INTO test VALUES (2, 1.0);
> +        INSERT INTO test VALUES (3, true);
> +        SELECT s2, typeof(s2) FROM test ORDER BY s2;
> +    ]], {
> +        -- <18.1>
> +        true,"boolean",1,"integer",1,"double"
> +        -- </18.1>
> +    })
> +
4. I think it was decided that we should create new file
for each new test.

5. I think, it is worth to add result of SELECT where index
was used. The example we can see in text of the issue.
Also, we should mention comewhere in the comments, that the
result should be the same in case index was used and in case
it wasn't.

>  test:finish_test()
> -- 
> 2.21.0 (Apple Git-122)
> 

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Hi! Thanks for the review.

Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4697-scalar-bug
Issue: https://github.com/tarantool/tarantool/issues/4697

@ChangeLog
- Fix the bug with wrong sorter order of boolean and integer
within SCALAR.

> On Apr 15, 2020, at 22:41, Mergen Imeev <imeevma@tarantool.org> wrote:
> 
> Hi! Thank you for the patch! See 5 small comments below.
> 
> On Mon, Apr 13, 2020 at 07:11:05AM +0300, Roman Khabibov wrote:
>> Make comparison rules in the sql sorter correct for number and
>> boolean. Boolean should always be greater than any number.
> 1. I think you should say that values compared here using
> SCALAR rules. It makes no sence otherwise, since boolean
> cannot be compared with number.
Fixed.

> 2. Isn't boolean always less that any number?
https://www.tarantool.io/en/doc/1.10/book/box/data_model/
See the "Indexed field types" table.

>> 
>> Closes #4697
>> ---
> 3. Please include links to issue and branch. Also,
> ChangeLog is missing.
>> src/box/sql/vdbeaux.c      |  9 ++++++---
>> test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
>> 2 files changed, 23 insertions(+), 4 deletions(-)
>> 
>> 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/sort.test.lua b/test/sql-tap/sort.test.lua
>> index 36074d6ef..15b397f1d 100755
>> --- a/test/sql-tap/sort.test.lua
>> +++ b/test/sql-tap/sort.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> test = require("sqltester")
>> -test:plan(60)
>> +test:plan(61)
>> 
>> --!./tcltestrunner.lua
>> -- 2001 September 15.
>> @@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
>>         })
>> 
>> end
>> +
>> +-- gh-4679 Make sure that boolean > any number within scalar.
>> +test:do_execsql_test(
>> +    18.1,
>> +    [[
>> +        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
>> +        INSERT INTO test VALUES (0, 1);
>> +        INSERT INTO test VALUES (2, 1.0);
>> +        INSERT INTO test VALUES (3, true);
>> +        SELECT s2, typeof(s2) FROM test ORDER BY s2;
>> +    ]], {
>> +        -- <18.1>
>> +        true,"boolean",1,"integer",1,"double"
>> +        -- </18.1>
>> +    })
>> +
> 4. I think it was decided that we should create new file
> for each new test.
> 
> 5. I think, it is worth to add result of SELECT where index
> was used. The example we can see in text of the issue.
> Also, we should mention comewhere in the comments, that the
> result should be the same in case index was used and in case
> it wasn't.
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..159e91fca
--- /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 > 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()

>> test:finish_test()
>> -- 
>> 2.21.0 (Apple Git-122)

commit 9ea60156e72dc45dbea66c9e614769b8d8677e6f (HEAD -> romanhabibov/gh-4697-scalar-bug)
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date:   Mon Apr 13 05:03:54 2020 +0300

    sql: fix number and boolean SCALAR sorting rules
    
    Make comparison SCALAR rules in the sql sorter correct for number
    and boolean. Boolean should always be greater than any number.
    
    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..159e91fca
--- /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 > 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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Mergen Imeev]

Hi! Thank you for fixes. See one comment below.

On Thu, Apr 16, 2020 at 03:34:29AM +0300, Roman Khabibov wrote:
> Hi! Thanks for the review.
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4697-scalar-bug
> Issue: https://github.com/tarantool/tarantool/issues/4697
> 
> @ChangeLog
> - Fix the bug with wrong sorter order of boolean and integer
> within SCALAR.
> 
> > On Apr 15, 2020, at 22:41, Mergen Imeev <imeevma@tarantool.org> wrote:
> > 
> > Hi! Thank you for the patch! See 5 small comments below.
> > 
> > On Mon, Apr 13, 2020 at 07:11:05AM +0300, Roman Khabibov wrote:
> >> Make comparison rules in the sql sorter correct for number and
> >> boolean. Boolean should always be greater than any number.
> > 1. I think you should say that values compared here using
> > SCALAR rules. It makes no sence otherwise, since boolean
> > cannot be compared with number.
> Fixed.
> 
> > 2. Isn't boolean always less that any number?
> https://www.tarantool.io/en/doc/1.10/book/box/data_model/
> See the "Indexed field types" table.
1. Not sure about this table, I cannot see where is says that
boolean is greater than number. But, we can see the order if
we look at 'enum mp_class' in tuple_compare.cc. It says, that
nil < boolean < number < string < varbinary and so on.

Also, if we execute these commands:

box.execute('create table t(i scalar primary key);')
box.execute('insert into t values (true), (1), (1.5);')
box.execute('select * from t where i > true;')

we will receive this result:
---
- metadata:
  - name: I
    type: scalar
  rows:
  - [1]
  - [1.5]
...

It shows that any number is more that any boolean.

Actually, in you tests order in the results also show this.

> 
> >> 
> >> Closes #4697
> >> ---
> > 3. Please include links to issue and branch. Also,
> > ChangeLog is missing.
> >> src/box/sql/vdbeaux.c      |  9 ++++++---
> >> test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
> >> 2 files changed, 23 insertions(+), 4 deletions(-)
> >> 
> >> 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/sort.test.lua b/test/sql-tap/sort.test.lua
> >> index 36074d6ef..15b397f1d 100755
> >> --- a/test/sql-tap/sort.test.lua
> >> +++ b/test/sql-tap/sort.test.lua
> >> @@ -1,6 +1,6 @@
> >> #!/usr/bin/env tarantool
> >> test = require("sqltester")
> >> -test:plan(60)
> >> +test:plan(61)
> >> 
> >> --!./tcltestrunner.lua
> >> -- 2001 September 15.
> >> @@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
> >>         })
> >> 
> >> end
> >> +
> >> +-- gh-4679 Make sure that boolean > any number within scalar.
> >> +test:do_execsql_test(
> >> +    18.1,
> >> +    [[
> >> +        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
> >> +        INSERT INTO test VALUES (0, 1);
> >> +        INSERT INTO test VALUES (2, 1.0);
> >> +        INSERT INTO test VALUES (3, true);
> >> +        SELECT s2, typeof(s2) FROM test ORDER BY s2;
> >> +    ]], {
> >> +        -- <18.1>
> >> +        true,"boolean",1,"integer",1,"double"
> >> +        -- </18.1>
> >> +    })
> >> +
> > 4. I think it was decided that we should create new file
> > for each new test.
> > 
> > 5. I think, it is worth to add result of SELECT where index
> > was used. The example we can see in text of the issue.
> > Also, we should mention comewhere in the comments, that the
> > result should be the same in case index was used and in case
> > it wasn't.
> 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..159e91fca
> --- /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 > 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()
> 
> >> test:finish_test()
> >> -- 
> >> 2.21.0 (Apple Git-122)
> 
> commit 9ea60156e72dc45dbea66c9e614769b8d8677e6f (HEAD -> romanhabibov/gh-4697-scalar-bug)
> Author: Roman Khabibov <roman.habibov@tarantool.org>
> Date:   Mon Apr 13 05:03:54 2020 +0300
> 
>     sql: fix number and boolean SCALAR sorting rules
>     
>     Make comparison SCALAR rules in the sql sorter correct for number
>     and boolean. Boolean should always be greater than any number.
>     
>     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..159e91fca
> --- /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 > 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()
> 
> 

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Hi! Thanks for the review.

> On Apr 16, 2020, at 11:17, Mergen Imeev <imeevma@tarantool.org> wrote:
> 
> Hi! Thank you for fixes. See one comment below.
> 
> On Thu, Apr 16, 2020 at 03:34:29AM +0300, Roman Khabibov wrote:
>> Hi! Thanks for the review.
>> 
>> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4697-scalar-bug
>> Issue: https://github.com/tarantool/tarantool/issues/4697
>> 
>> @ChangeLog
>> - Fix the bug with wrong sorter order of boolean and integer
>> within SCALAR.
>> 
>>> On Apr 15, 2020, at 22:41, Mergen Imeev <imeevma@tarantool.org> wrote:
>>> 
>>> Hi! Thank you for the patch! See 5 small comments below.
>>> 
>>> On Mon, Apr 13, 2020 at 07:11:05AM +0300, Roman Khabibov wrote:
>>>> Make comparison rules in the sql sorter correct for number and
>>>> boolean. Boolean should always be greater than any number.
>>> 1. I think you should say that values compared here using
>>> SCALAR rules. It makes no sence otherwise, since boolean
>>> cannot be compared with number.
>> Fixed.
>> 
>>> 2. Isn't boolean always less that any number?
>> https://www.tarantool.io/en/doc/1.10/book/box/data_model/
>> See the "Indexed field types" table.
> 1. Not sure about this table, I cannot see where is says that
> boolean is greater than number. But, we can see the order if
> we look at 'enum mp_class' in tuple_compare.cc. It says, that
> nil < boolean < number < string < varbinary and so on.
> 
> Also, if we execute these commands:
> 
> box.execute('create table t(i scalar primary key);')
> box.execute('insert into t values (true), (1), (1.5);')
> box.execute('select * from t where i > true;')
> 
> we will receive this result:
> ---
> - metadata:
>  - name: I
>    type: scalar
>  rows:
>  - [1]
>  - [1.5]
> ...
> 
> It shows that any number is more that any boolean.
> 
> Actually, in you tests order in the results also show this.
Sorry, I mixed up the sorting order. Fixed.

>> 
>>>> 
>>>> Closes #4697
>>>> ---
>>> 3. Please include links to issue and branch. Also,
>>> ChangeLog is missing.
>>>> src/box/sql/vdbeaux.c      |  9 ++++++---
>>>> test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
>>>> 2 files changed, 23 insertions(+), 4 deletions(-)
>>>> 
>>>> 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/sort.test.lua b/test/sql-tap/sort.test.lua
>>>> index 36074d6ef..15b397f1d 100755
>>>> --- a/test/sql-tap/sort.test.lua
>>>> +++ b/test/sql-tap/sort.test.lua
>>>> @@ -1,6 +1,6 @@
>>>> #!/usr/bin/env tarantool
>>>> test = require("sqltester")
>>>> -test:plan(60)
>>>> +test:plan(61)
>>>> 
>>>> --!./tcltestrunner.lua
>>>> -- 2001 September 15.
>>>> @@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
>>>>        })
>>>> 
>>>> end
>>>> +
>>>> +-- gh-4679 Make sure that boolean > any number within scalar.
>>>> +test:do_execsql_test(
>>>> +    18.1,
>>>> +    [[
>>>> +        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
>>>> +        INSERT INTO test VALUES (0, 1);
>>>> +        INSERT INTO test VALUES (2, 1.0);
>>>> +        INSERT INTO test VALUES (3, true);
>>>> +        SELECT s2, typeof(s2) FROM test ORDER BY s2;
>>>> +    ]], {
>>>> +        -- <18.1>
>>>> +        true,"boolean",1,"integer",1,"double"
>>>> +        -- </18.1>
>>>> +    })
>>>> +
>>> 4. I think it was decided that we should create new file
>>> for each new test.
>>> 
>>> 5. I think, it is worth to add result of SELECT where index
>>> was used. The example we can see in text of the issue.
>>> Also, we should mention comewhere in the comments, that the
>>> result should be the same in case index was used and in case
>>> it wasn't.
>> 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..159e91fca
>> --- /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 > 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()
>> 
>>>> test:finish_test()
>>>> -- 
>>>> 2.21.0 (Apple Git-122)
>> 
>> commit 9ea60156e72dc45dbea66c9e614769b8d8677e6f (HEAD -> romanhabibov/gh-4697-scalar-bug)
>> Author: Roman Khabibov <roman.habibov@tarantool.org>
>> Date:   Mon Apr 13 05:03:54 2020 +0300
>> 
>>    sql: fix number and boolean SCALAR sorting rules
>> 
>>    Make comparison SCALAR rules in the sql sorter correct for number
>>    and boolean. Boolean should always be greater than any number.
>> 
>>    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..159e91fca
>> --- /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 > 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(

commit 8a2133d84b88a050181d858d6109abf3635a121b (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 number and boolean SCALAR sorting rules
    
    Make comparison SCALAR rules in the sql sorter correct for number
    and boolean. Boolean should always follow before any number.
    
    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..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.
+--
+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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Mergen Imeev]

Hi! Thank you for the fixes. I think that the your
commit-message and comments sounds quite strange, but I am
not sure that I am qualified enough to say that for sure.
Please, send this patch to NIkita. Other than that, LGTM.

On Fri, Apr 17, 2020 at 03:48:57AM +0300, Roman Khabibov wrote:
> Hi! Thanks for the review.
> 
> > On Apr 16, 2020, at 11:17, Mergen Imeev <imeevma@tarantool.org> wrote:
> > 
> > Hi! Thank you for fixes. See one comment below.
> > 
> > On Thu, Apr 16, 2020 at 03:34:29AM +0300, Roman Khabibov wrote:
> >> Hi! Thanks for the review.
> >> 
> >> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4697-scalar-bug
> >> Issue: https://github.com/tarantool/tarantool/issues/4697
> >> 
> >> @ChangeLog
> >> - Fix the bug with wrong sorter order of boolean and integer
> >> within SCALAR.
> >> 
> >>> On Apr 15, 2020, at 22:41, Mergen Imeev <imeevma@tarantool.org> wrote:
> >>> 
> >>> Hi! Thank you for the patch! See 5 small comments below.
> >>> 
> >>> On Mon, Apr 13, 2020 at 07:11:05AM +0300, Roman Khabibov wrote:
> >>>> Make comparison rules in the sql sorter correct for number and
> >>>> boolean. Boolean should always be greater than any number.
> >>> 1. I think you should say that values compared here using
> >>> SCALAR rules. It makes no sence otherwise, since boolean
> >>> cannot be compared with number.
> >> Fixed.
> >> 
> >>> 2. Isn't boolean always less that any number?
> >> https://www.tarantool.io/en/doc/1.10/book/box/data_model/
> >> See the "Indexed field types" table.
> > 1. Not sure about this table, I cannot see where is says that
> > boolean is greater than number. But, we can see the order if
> > we look at 'enum mp_class' in tuple_compare.cc. It says, that
> > nil < boolean < number < string < varbinary and so on.
> > 
> > Also, if we execute these commands:
> > 
> > box.execute('create table t(i scalar primary key);')
> > box.execute('insert into t values (true), (1), (1.5);')
> > box.execute('select * from t where i > true;')
> > 
> > we will receive this result:
> > ---
> > - metadata:
> >  - name: I
> >    type: scalar
> >  rows:
> >  - [1]
> >  - [1.5]
> > ...
> > 
> > It shows that any number is more that any boolean.
> > 
> > Actually, in you tests order in the results also show this.
> Sorry, I mixed up the sorting order. Fixed.
> 
> >> 
> >>>> 
> >>>> Closes #4697
> >>>> ---
> >>> 3. Please include links to issue and branch. Also,
> >>> ChangeLog is missing.
> >>>> src/box/sql/vdbeaux.c      |  9 ++++++---
> >>>> test/sql-tap/sort.test.lua | 18 +++++++++++++++++-
> >>>> 2 files changed, 23 insertions(+), 4 deletions(-)
> >>>> 
> >>>> 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/sort.test.lua b/test/sql-tap/sort.test.lua
> >>>> index 36074d6ef..15b397f1d 100755
> >>>> --- a/test/sql-tap/sort.test.lua
> >>>> +++ b/test/sql-tap/sort.test.lua
> >>>> @@ -1,6 +1,6 @@
> >>>> #!/usr/bin/env tarantool
> >>>> test = require("sqltester")
> >>>> -test:plan(60)
> >>>> +test:plan(61)
> >>>> 
> >>>> --!./tcltestrunner.lua
> >>>> -- 2001 September 15.
> >>>> @@ -923,4 +923,20 @@ box.internal.sql_create_function("cksum", cksum)
> >>>>        })
> >>>> 
> >>>> end
> >>>> +
> >>>> +-- gh-4679 Make sure that boolean > any number within scalar.
> >>>> +test:do_execsql_test(
> >>>> +    18.1,
> >>>> +    [[
> >>>> +        CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR);
> >>>> +        INSERT INTO test VALUES (0, 1);
> >>>> +        INSERT INTO test VALUES (2, 1.0);
> >>>> +        INSERT INTO test VALUES (3, true);
> >>>> +        SELECT s2, typeof(s2) FROM test ORDER BY s2;
> >>>> +    ]], {
> >>>> +        -- <18.1>
> >>>> +        true,"boolean",1,"integer",1,"double"
> >>>> +        -- </18.1>
> >>>> +    })
> >>>> +
> >>> 4. I think it was decided that we should create new file
> >>> for each new test.
> >>> 
> >>> 5. I think, it is worth to add result of SELECT where index
> >>> was used. The example we can see in text of the issue.
> >>> Also, we should mention comewhere in the comments, that the
> >>> result should be the same in case index was used and in case
> >>> it wasn't.
> >> 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..159e91fca
> >> --- /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 > 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()
> >> 
> >>>> test:finish_test()
> >>>> -- 
> >>>> 2.21.0 (Apple Git-122)
> >> 
> >> commit 9ea60156e72dc45dbea66c9e614769b8d8677e6f (HEAD -> romanhabibov/gh-4697-scalar-bug)
> >> Author: Roman Khabibov <roman.habibov@tarantool.org>
> >> Date:   Mon Apr 13 05:03:54 2020 +0300
> >> 
> >>    sql: fix number and boolean SCALAR sorting rules
> >> 
> >>    Make comparison SCALAR rules in the sql sorter correct for number
> >>    and boolean. Boolean should always be greater than any number.
> >> 
> >>    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..159e91fca
> >> --- /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 > 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(
> 
> commit 8a2133d84b88a050181d858d6109abf3635a121b (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 number and boolean SCALAR sorting rules
>     
>     Make comparison SCALAR rules in the sql sorter correct for number
>     and boolean. Boolean should always follow before any number.
>     
>     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..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.
> +--
> +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()
> 

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Timur Safin]

As a random bypasser I could not resist and not add my 5 kopecks (see below).

: -----Original Message-----
: From: Mergen Imeev <imeevma@tarantool.org>
: 
: Hi! Thank you for the fixes. I think that the your
: commit-message and comments sounds quite strange, but I am
: not sure that I am qualified enough to say that for sure.
: Please, send this patch to NIkita. Other than that, LGTM.
: 


: > commit 8a2133d84b88a050181d858d6109abf3635a121b (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 number and boolean SCALAR sorting rules
: >
: >     Make comparison SCALAR rules in the sql sorter correct for number
: >     and boolean. Boolean should always follow before any number.
: >
: >     Closes #4697
: >

Agreed with Mergen that message is confusing, I'd suggest to do this simple replacement to make it clearer 

s/follow before/precede/g

Regards,
Timur

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

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).
> 
> : -----Original Message-----
> : From: Mergen Imeev <imeevma@tarantool.org>
> : 
> : Hi! Thank you for the fixes. I think that the your
> : commit-message and comments sounds quite strange, but I am
> : not sure that I am qualified enough to say that for sure.
> : Please, send this patch to NIkita. Other than that, LGTM.
> : 
> 
> 
> : > commit 8a2133d84b88a050181d858d6109abf3635a121b (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 number and boolean SCALAR sorting rules
> : >
> : >     Make comparison SCALAR rules in the sql sorter correct for number
> : >     and boolean. Boolean should always follow before any number.
> : >
> : >     Closes #4697
> : >
> 
> Agreed with Mergen that message is confusing, I'd suggest to do this simple replacement to make it clearer 
> 
> s/follow before/precede/g
> 
> Regards,
> Timur
> 
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 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..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.
+--
+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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Timur Safin]

: -----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 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.

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

> 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 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 <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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Timur Safin]

If you would be asking me then LGTM now

Timur

: -----Original Message-----
: From: Roman Khabibov <roman.habibov@tarantool.org>
: Sent: Saturday, April 18, 2020 5:46 AM
: 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
: 
: 
: > 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 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 <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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Hi! Kirill, two LGTMs.

> On Apr 18, 2020, at 12:18, Timur Safin <tsafin@tarantool.org> wrote:
> 
> If you would be asking me then LGTM now
> 
> Timur
> 
> : -----Original Message-----
> : From: Roman Khabibov <roman.habibov@tarantool.org>
> : Sent: Saturday, April 18, 2020 5:46 AM
> : 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
> : 
> : 
> : > 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 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 <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()
> 
> 

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Nikita Pettik]

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@tarantool.org> wrote:
> > 
> > If you would be asking me then LGTM now
> > 
> > Timur
> > 
> > : -----Original Message-----
> > : From: Roman Khabibov <roman.habibov@tarantool.org>
> > : Sent: Saturday, April 18, 2020 5:46 AM
> > : 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
> > : 
> > : 
> > : > 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 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 <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()
> > 
> > 
> 

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Hi! Yes.

> On Apr 20, 2020, at 04:05, Nikita Pettik <korablev@tarantool.org> wrote:
> 
> 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@tarantool.org> wrote:
>>> 
>>> If you would be asking me then LGTM now
>>> 
>>> Timur
>>> 
>>> : -----Original Message-----
>>> : From: Roman Khabibov <roman.habibov@tarantool.org>
>>> : Sent: Saturday, April 18, 2020 5:46 AM
>>> : 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
>>> : 
>>> : 
>>> : > 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 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 <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()
>>> 
>>> 
>> 

commit 48530ef30fbbb3c7983e75385adbc43448fb9732 (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 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()

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Nikita Pettik]

On 20 Apr 23:28, Roman Khabibov wrote:
> 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;

Nit: please, split this into two branches. This makes code flow
more clear. It is not so easy to parse ternary operator with
complex condition.

> 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

Please, use more specific test name. Like: gh-4697-scalar-bool-sort-cmp.test.lua
Also, now we can write tests using only SQL format. Let's strive to use
it whenever it is possible.

> @@ -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);

Check also NULLs as values (since NULLs appear in the same code path).

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Roman Khabibov]

Hi! Thanks for the review.

> On Apr 21, 2020, at 02:57, Nikita Pettik <korablev@tarantool.org> wrote:
> 
> On 20 Apr 23:28, Roman Khabibov wrote:
>> 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;
> 
> Nit: please, split this into two branches. This makes code flow
> more clear. It is not so easy to parse ternary operator with
> complex condition.
@@ -3053,8 +3053,11 @@ sqlVdbeCompareMsgpack(const char **key1,
 			} else if ((pKey2->flags & MEM_Real) != 0) {
 				rc = double_compare_uint64(pKey2->u.r,
 							   mem1.u.u, -1);
+			} else if ((pKey2->flags & MEM_Null) != 0 ||
+				     (pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3071,8 +3074,11 @@ sqlVdbeCompareMsgpack(const char **key1,
 			} else if (pKey2->flags & MEM_Real) {
 				rc = double_compare_uint64(-pKey2->u.r,
 							   -mem1.u.i, 1);
+			} else if ((pKey2->flags & MEM_Null) != 0 ||
+				     (pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3096,7 +3102,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
> 
> Please, use more specific test name. Like: gh-4697-scalar-bool-sort-cmp.test.lua
> Also, now we can write tests using only SQL format. Let's strive to use
> it whenever it is possible.
> 
>> @@ -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);
> 
> Check also NULLs as values (since NULLs appear in the same code path).
diff --git a/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql b/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql
new file mode 100755
index 000000000..faca6819d
--- /dev/null
+++ b/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql
@@ -0,0 +1,13 @@
+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);
+INSERT INTO test VALUES (3, NULL, NULL);
+
+--
+-- 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.
+--
+SELECT s2, typeof(s2) FROM test ORDER BY s2;
+SELECT s3, typeof(s3) FROM test ORDER BY s3;


commit 2dad4688dac14faded9547d4b24065b8d2fcf1a9 (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 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..310cec3ed 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -3053,8 +3053,11 @@ sqlVdbeCompareMsgpack(const char **key1,
 			} else if ((pKey2->flags & MEM_Real) != 0) {
 				rc = double_compare_uint64(pKey2->u.r,
 							   mem1.u.u, -1);
+			} else if ((pKey2->flags & MEM_Null) != 0 ||
+				     (pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3071,8 +3074,11 @@ sqlVdbeCompareMsgpack(const char **key1,
 			} else if (pKey2->flags & MEM_Real) {
 				rc = double_compare_uint64(-pKey2->u.r,
 							   -mem1.u.i, 1);
+			} else if ((pKey2->flags & MEM_Null) != 0 ||
+				     (pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3096,7 +3102,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/gh-4697-scalar-bool-sort-cmp.result b/test/sql/gh-4697-scalar-bool-sort-cmp.result
new file mode 100644
index 000000000..2423cd894
--- /dev/null
+++ b/test/sql/gh-4697-scalar-bool-sort-cmp.result
@@ -0,0 +1,53 @@
+-- test-run result file version 2
+CREATE TABLE test (s1 INTEGER PRIMARY KEY, s2 SCALAR UNIQUE, s3 SCALAR);
+ | ---
+ | - row_count: 1
+ | ...
+INSERT INTO test VALUES (0, 1, 1);
+ | ---
+ | - row_count: 1
+ | ...
+INSERT INTO test VALUES (1, 1.1, 1.1);
+ | ---
+ | - row_count: 1
+ | ...
+INSERT INTO test VALUES (2, true, true);
+ | ---
+ | - row_count: 1
+ | ...
+INSERT INTO test VALUES (3, NULL, NULL);
+ | ---
+ | - row_count: 1
+ | ...
+
+--
+-- 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.
+--
+SELECT s2, typeof(s2) FROM test ORDER BY s2;
+ | ---
+ | - metadata:
+ |   - name: S2
+ |     type: scalar
+ |   - name: typeof(s2)
+ |     type: string
+ |   rows:
+ |   - [null, 'boolean']
+ |   - [true, 'boolean']
+ |   - [1, 'integer']
+ |   - [1.1, 'double']
+ | ...
+SELECT s3, typeof(s3) FROM test ORDER BY s3;
+ | ---
+ | - metadata:
+ |   - name: S3
+ |     type: scalar
+ |   - name: typeof(s3)
+ |     type: string
+ |   rows:
+ |   - [null, 'boolean']
+ |   - [true, 'boolean']
+ |   - [1, 'integer']
+ |   - [1.1, 'double']
+ | ...
diff --git a/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql b/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql
new file mode 100755
index 000000000..faca6819d
--- /dev/null
+++ b/test/sql/gh-4697-scalar-bool-sort-cmp.test.sql
@@ -0,0 +1,13 @@
+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);
+INSERT INTO test VALUES (3, NULL, NULL);
+
+--
+-- 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.
+--
+SELECT s2, typeof(s2) FROM test ORDER BY s2;
+SELECT s3, typeof(s3) FROM test ORDER BY s3;

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Nikita Pettik]

On 24 Apr 16:03, Roman Khabibov wrote:
> Hi! Thanks for the review.
> 
> > On Apr 21, 2020, at 02:57, Nikita Pettik <korablev@tarantool.org> wrote:
> > 
> > On 20 Apr 23:28, Roman Khabibov wrote:

Pushed to master and 2.3 with fixes attached below. Changelogs are
not updated due to the lack of drafts on github. Branch has been dropped.

diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 310cec3ed..71f694b20 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -3053,8 +3053,9 @@ sqlVdbeCompareMsgpack(const char **key1,
                        } else if ((pKey2->flags & MEM_Real) != 0) {
                                rc = double_compare_uint64(pKey2->u.r,
                                                           mem1.u.u, -1);
-                       } else if ((pKey2->flags & MEM_Null) != 0 ||
-                                    (pKey2->flags & MEM_Bool) != 0) {
+                       } else if ((pKey2->flags & MEM_Null) != 0) {
+                               rc = 1;
+                       } else if ((pKey2->flags & MEM_Bool) != 0) {
                                rc = 1;
                        } else {
                                rc = -1;
@@ -3074,8 +3075,9 @@ sqlVdbeCompareMsgpack(const char **key1,
                        } else if (pKey2->flags & MEM_Real) {
                                rc = double_compare_uint64(-pKey2->u.r,
                                                           -mem1.u.i, 1);
-                       } else if ((pKey2->flags & MEM_Null) != 0 ||
-                                    (pKey2->flags & MEM_Bool) != 0) {
+                       } else if ((pKey2->flags & MEM_Null) != 0) {
+                               rc = 1;
+                       } else if ((pKey2->flags & MEM_Bool) != 0) {
                                rc = 1;
                        } else {
                                rc = -1;
@@ -3101,9 +3103,12 @@ sqlVdbeCompareMsgpack(const char **key1,
                                } else if (mem1.u.r > pKey2->u.r) {
                                        rc = +1;
                                }
+                       } else if ((pKey2->flags & MEM_Null) != 0) {
+                               rc = 1;
+                       } else if ((pKey2->flags & MEM_Bool) != 0) {
+                               rc = 1;
                        } else {
-                               rc = (pKey2->flags & MEM_Null) != 0 ||
-                                    (pKey2->flags & MEM_Bool) != 0 ? 1 : -1;
+                               rc = -1;
                        }
                        break;

Re: [Tarantool-patches] [PATCH] sql: fix number and boolean sorting rules

[Nikita Pettik]

On 27 Apr 01:34, Nikita Pettik wrote:
> On 24 Apr 16:03, Roman Khabibov wrote:
> > Hi! Thanks for the review.
> > 
> > > On Apr 21, 2020, at 02:57, Nikita Pettik <korablev@tarantool.org> wrote:
> > > 
> > > On 20 Apr 23:28, Roman Khabibov wrote:
> 
> Pushed to master and 2.3 with fixes attached below. Changelogs are
> not updated due to the lack of drafts on github. Branch has been dropped.

Pushed to 2.4 as well. Updated changelogs (I failed to find it in
mailing list so wrote it myself).
 
> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index 310cec3ed..71f694b20 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -3053,8 +3053,9 @@ sqlVdbeCompareMsgpack(const char **key1,
>                         } else if ((pKey2->flags & MEM_Real) != 0) {
>                                 rc = double_compare_uint64(pKey2->u.r,
>                                                            mem1.u.u, -1);
> -                       } else if ((pKey2->flags & MEM_Null) != 0 ||
> -                                    (pKey2->flags & MEM_Bool) != 0) {
> +                       } else if ((pKey2->flags & MEM_Null) != 0) {
> +                               rc = 1;
> +                       } else if ((pKey2->flags & MEM_Bool) != 0) {
>                                 rc = 1;
>                         } else {
>                                 rc = -1;
> @@ -3074,8 +3075,9 @@ sqlVdbeCompareMsgpack(const char **key1,
>                         } else if (pKey2->flags & MEM_Real) {
>                                 rc = double_compare_uint64(-pKey2->u.r,
>                                                            -mem1.u.i, 1);
> -                       } else if ((pKey2->flags & MEM_Null) != 0 ||
> -                                    (pKey2->flags & MEM_Bool) != 0) {
> +                       } else if ((pKey2->flags & MEM_Null) != 0) {
> +                               rc = 1;
> +                       } else if ((pKey2->flags & MEM_Bool) != 0) {
>                                 rc = 1;
>                         } else {
>                                 rc = -1;
> @@ -3101,9 +3103,12 @@ sqlVdbeCompareMsgpack(const char **key1,
>                                 } else if (mem1.u.r > pKey2->u.r) {
>                                         rc = +1;
>                                 }
> +                       } else if ((pKey2->flags & MEM_Null) != 0) {
> +                               rc = 1;
> +                       } else if ((pKey2->flags & MEM_Bool) != 0) {
> +                               rc = 1;
>                         } else {
> -                               rc = (pKey2->flags & MEM_Null) != 0 ||
> -                                    (pKey2->flags & MEM_Bool) != 0 ? 1 : -1;
> +                               rc = -1;
>                         }
>                         break;
>