From 72ce442c933fb4c97a05789cf070d4fb480b84bc Mon Sep 17 00:00:00 2001
From: Roman Khabibov <roman.habibov@tarantool.org>
Date: Mon, 13 Apr 2020 05:03:54 +0300
Subject: [PATCH] sql: fix sorting rules for values of SCALAR type

Function implementing comparison during VDBE sorting routine
(sqlVdbeCompareMsgpack) did not account values of boolean type in some
cases. Let's fix it so that booleans always precede numbers if they are
sorted in ascending order.

Closes #4697
---
 src/box/sql/vdbeaux.c                         | 18 +++++--
 test/sql/gh-4697-scalar-bool-sort-cmp.result  | 53 +++++++++++++++++++
 .../sql/gh-4697-scalar-bool-sort-cmp.test.sql | 13 +++++
 3 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100644 test/sql/gh-4697-scalar-bool-sort-cmp.result
 create mode 100755 test/sql/gh-4697-scalar-bool-sort-cmp.test.sql

diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index b88726ea1c..71f694b20a 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -3053,8 +3053,12 @@ 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) {
+				rc = 1;
+			} else if ((pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3071,8 +3075,12 @@ 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) {
+				rc = 1;
+			} else if ((pKey2->flags & MEM_Bool) != 0) {
+				rc = 1;
 			} else {
-				rc = (pKey2->flags & MEM_Null) ? +1 : -1;
+				rc = -1;
 			}
 			break;
 		}
@@ -3095,8 +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) ? +1 : -1;
+				rc = -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 0000000000..2423cd8948
--- /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 0000000000..faca6819d2
--- /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;
-- 
GitLab