Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: forbid different sorting orders in ORDER BY
@ 2019-03-11 15:44 Ivan Koptelov
  2019-03-12 14:25 ` [tarantool-patches] " n.pettik
  2019-03-19 11:25 ` Kirill Yukhin
  0 siblings, 2 replies; 7+ messages in thread
From: Ivan Koptelov @ 2019-03-11 15:44 UTC (permalink / raw)
  To: tarantool-patches; +Cc: korablev, Ivan Koptelov

Without multi-directional iterators (to be introduced in #3309)
in most cases (except those when internal VDBE merge sorter is
used) ORDER BY with different specified ordered leads to wrong
result. So for now (till #3309 is resolved) different sort
orders are forbidden.

Close #4038

@TarantoolBot document
Title: different sorting orders in ORDER BY are forbidden now
The error will be raised if different sorting orders are
encountered.
---
Branch https://github.com/tarantool/tarantool/tree/sudobobo/gh-4038-ban-diff-orders-in-ORDER-BY
Issue https://github.com/tarantool/tarantool/issues/4038

 src/box/sql/expr.c                   |  15 +++
 src/box/sql/parse.y                  |   5 +-
 src/box/sql/sqlInt.h                 |  14 +++
 test/sql-tap/e_select1.test.lua      | 165 ++++++++++++++++++---------
 test/sql-tap/orderby1.test.lua       |  67 ++++++-----
 test/sql-tap/orderby2.test.lua       |  15 ++-
 test/sql-tap/orderby6.test.lua       | 125 ++++++++++++--------
 test/sql-tap/select1.test.lua        |  15 ++-
 test/sql-tap/select4.test.lua        |   3 +-
 test/sql-tap/sort.test.lua           |  35 +++---
 test/sql-tap/tkt-4dd95f6943.test.lua | 128 ++++++++++++---------
 test/sql-tap/tkt-b75a9ca6b0.test.lua |  33 +++---
 test/sql-tap/tkt-ba7cbfaedc.test.lua |  24 +++-
 test/sql-tap/where2.test.lua         |  20 +++-
 14 files changed, 426 insertions(+), 238 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a75f23756..85719e3a1 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1809,6 +1809,21 @@ sqlExprListSetSortOrder(struct ExprList *p, enum sort_order sort_order)
 	p->a[p->nExpr - 1].sort_order = sort_order;
 }
 
+void
+sql_check_sort_orders(ExprList * expr_list, Parse *parse) {
+	if(expr_list == NULL)
+		return;
+	for (int i = 0; i < expr_list->nExpr; i++) {
+		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
+		if (expr_list->a[i].sort_order != expr_list->a[0].sort_order) {
+			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
+				 "different sorting orders");
+			sql_parser_error(parse);
+			return;
+		}
+	}
+}
+
 /*
  * Set the ExprList.a[].zName element of the most recently added item
  * on the expression list.
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 996f55d37..95d6962f9 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -661,7 +661,10 @@ using_opt(U) ::= .                        {U = 0;}
 %destructor sortlist {sql_expr_list_delete(pParse->db, $$);}
 
 orderby_opt(A) ::= .                          {A = 0;}
-orderby_opt(A) ::= ORDER BY sortlist(X).      {A = X;}
+orderby_opt(A) ::= ORDER BY sortlist(X). {
+  sql_check_sort_orders(X, pParse);
+  A = X;
+}
 sortlist(A) ::= sortlist(A) COMMA expr(Y) sortorder(Z). {
   A = sql_expr_list_append(pParse->db,A,Y.pExpr);
   sqlExprListSetSortOrder(A,Z);
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1d8fae5b0..06818d9e1 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3237,6 +3237,20 @@ ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *);
  */
 void sqlExprListSetSortOrder(ExprList *, enum sort_order sort_order);
 
+/**
+ * Check if sorting orders are the same in ORDER BY and rise an
+ * error if they are not.
+ *
+ * In future, we will support different sorting orders in
+ * ORDER BY (e.g. ORDER BY col1 ASC, col2 DESC) and remove this
+ * check (see ticket #3309).
+ * @param expr_list Expression list with  ORDER BY clause
+ * at the end.
+ * @param parse Parsing context.
+ */
+void
+sql_check_sort_orders(ExprList * expr_list, Parse *parse);
+
 void sqlExprListSetName(Parse *, ExprList *, Token *, int);
 void sqlExprListSetSpan(Parse *, ExprList *, ExprSpan *);
 u32 sqlExprListFlags(const ExprList *);
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index 75fe81e0a..8c58ee9b5 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -1739,69 +1739,126 @@ test:do_select_tests(
 --   Test cases e_select-8.3.* test the above. All 8.3 test cases are
 --   copies of 8.2 test cases with the explicit "ASC" removed.
 --
-test:do_select_tests(
-    "e_select-8",
+-- NOTE: some tests in the e_select_8.2 and 8.3 suites are
+-- expected to fail with "ORDER BY does not support different
+-- sorting orders" error. This behavior is temporary and
+-- corresponding tests must be fixed when different sorting
+-- orders will be allowed in ORDER BY. For details please see
+-- tickets #4038 and #3309.
+local test_cases = {
     {
-        {"2.1", "SELECT x,y,z FROM d1 ORDER BY x ASC, y ASC, z ASC", {
-            1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1
-        }},
-        {"2.2", "SELECT x,y,z FROM d1 ORDER BY x DESC, y DESC, z DESC", {
-            2,5,-1,2,4,93,1,5,-1,1,4,93,1,2,8,1,2,7,1,2,3,1,2,-20
-        }},
-        {"2.3", "SELECT x,y,z FROM d1 ORDER BY x DESC, y ASC, z DESC", {
-            2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1
-        }},
-        {"2.4", "SELECT x,y,z FROM d1 ORDER BY x DESC, y ASC, z ASC", {
-            2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1
-        }},
+        test = "SELECT x,y,z FROM d1 ORDER BY x ASC, y ASC, z ASC",
+        result = {0, {1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY x DESC, y DESC, z DESC",
+        result = {0, {2,5,-1,2,4,93,1,5,-1,1,4,93,1,2,8,1,2,7,1,2,3,1,2,-20,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY x DESC, y ASC, z DESC",
+        result = {1, "ORDER BY does not support different sorting orders"},
+        --result = {0, {2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1,}}
+    },
+    {
+        test =  "SELECT x,y,z FROM d1 ORDER BY x DESC, y ASC, z ASC",
+        result = {1, "ORDER BY does not support different sorting orders"},
+        --result = {0, {2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,}}
+    }
+}
 
-        {"3.1", "SELECT x,y,z FROM d1 ORDER BY x, y, z", {
-            1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1
-        }},
-        {"3.3", "SELECT x,y,z FROM d1 ORDER BY x DESC, y, z DESC", {
-            2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1
-        }},
-        {"3.4", "SELECT x,y,z FROM d1 ORDER BY x DESC, y, z", {
-            2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1
-        }},
-    })
+for i, test_case in ipairs(test_cases) do
+    test:do_catchsql_test(
+        "e_select-8.2." .. tostring(i),
+        test_case.test,
+        test_case.result
+    )
+end
+
+test_cases = {
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY x, y, z",
+        result = {0, {1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY x DESC, y, z DESC",
+        result = {1, "ORDER BY does not support different sorting orders"},
+        --result = {0, {2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY x DESC, y, z",
+        result = {1, "ORDER BY does not support different sorting orders"},
+        --result = {0, {2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,}}
+    },
+}
+
+for i, test_case in ipairs(test_cases) do
+    test:do_catchsql_test(
+        "e_select-8.3." .. tostring(i),
+        test_case.test,
+        test_case.result
+    )
+end
 
 -- EVIDENCE-OF: R-29779-04281 If the ORDER BY expression is a constant
 -- integer K then the expression is considered an alias for the K-th
 -- column of the result set (columns are numbered from left to right
 -- starting with 1).
+-- NOTE: some tests in the e_select_8.4 suite are expected to
+-- fail with "ORDER BY does not support different sorting orders"
+-- error. This behavior is temporary and corresponding tests must
+-- be fixed when different sorting orders will be allowed in
+-- ORDER BY. For details please see tickets #4038 and #3309.
 --
-test:do_select_tests(
-    "e_select-8.4",
+test_cases = {
     {
-        {"1", "SELECT x,y,z FROM d1 ORDER BY 1 ASC, 2 ASC, 3 ASC", {
-            1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1
-        }},
-        {"2", "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 DESC, 3 DESC", {
-            2,5,-1,2,4,93,1,5,-1,1,4,93,1,2,8,1,2,7,1,2,3,1,2,-20
-        }},
-        {"3", "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 ASC, 3 DESC", {
-            2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1
-        }},
-        {"4", "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 ASC, 3 ASC", {
-            2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1
-        }},
-        {"5", "SELECT x,y,z FROM d1 ORDER BY 1, 2, 3", {
-            1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1
-        }},
-        {"6", "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2, 3 DESC", {
-            2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1
-        }},
-        {"7", "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2, 3", {
-            2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1
-        }},
-        {"8", "SELECT z, x FROM d1 ORDER BY 2", {
-            3,1,8,1,7,1,-20,1,93,1,-1,1,-1,2,93,2
-        }},
-        {"9", "SELECT z, x FROM d1 ORDER BY 1", {
-            -20,1,-1,2,-1,1,3,1,7,1,8,1,93,2,93,1
-        }},
-    })
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 ASC, 2 ASC, 3 ASC",
+        result = {0, {1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 DESC, 3 DESC",
+        result = {0, {2,5,-1,2,4,93,1,5,-1,1,4,93,1,2,8,1,2,7,1,2,3,1,2,-20,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 ASC, 3 DESC",
+        result = {1, "ORDER BY does not support different sorting orders"}
+        --result = {0, {2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2 ASC, 3 ASC",
+        result = {1, "ORDER BY does not support different sorting orders"}
+        --result = {0, {1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1, 2, 3",
+        results = {0, {1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,2,4,93,2,5,-1}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2, 3 DESC",
+        result = {1, "ORDER BY does not support different sorting orders"}
+        --result = {0, {2,4,93,2,5,-1,1,2,8,1,2,7,1,2,3,1,2,-20,1,4,93,1,5,-1,}}
+    },
+    {
+        test = "SELECT x,y,z FROM d1 ORDER BY 1 DESC, 2, 3",
+        result = {1, "ORDER BY does not support different sorting orders"}
+        --result = {0, {2,4,93,2,5,-1,1,2,-20,1,2,3,1,2,7,1,2,8,1,4,93,1,5,-1,}}
+    },
+    {
+        test = "SELECT z, x FROM d1 ORDER BY 2",
+        result = {0, {3,1,8,1,7,1,-20,1,93,1,-1,1,-1,2,93,2,}}
+    },
+    {
+        test = "SELECT z, x FROM d1 ORDER BY 1",
+        result = {0, {-20,1,-1,2,-1,1,3,1,7,1,8,1,93,2,93,1,}}
+    },
+}
+
+for i, test_case in ipairs(test_cases) do
+    test:do_catchsql_test(
+        "e_select-8.4." .. tostring(i),
+        test_case.test,
+        test_case.result
+    )
+end
 
 -- EVIDENCE-OF: R-63286-51977 If the ORDER BY expression is an identifier
 -- that corresponds to the alias of one of the output columns, then the
diff --git a/test/sql-tap/orderby1.test.lua b/test/sql-tap/orderby1.test.lua
index ea03c494b..cb6636f63 100755
--- a/test/sql-tap/orderby1.test.lua
+++ b/test/sql-tap/orderby1.test.lua
@@ -17,6 +17,13 @@ test:plan(35)
 -- focus of this file is testing that the optimizations that disable
 -- ORDER BY clauses when the natural order of a query is correct.
 --
+-- NOTE: some tests in this file and also in another orderby test
+-- files are expected to fail with "ORDER BY does not support
+-- different sorting orders" error. This behavior is temporary
+-- and corresponding tests must be fixed when different sorting
+-- orders will be allowed in ORDER BY. For details please see
+-- tickets #4038 and #3309.
+--
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "orderby1"
@@ -155,24 +162,24 @@ test:do_test(
 test:do_test(
     "1.4a",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY title DESC, tn
         ]]
     end, {
         -- <1.4a>
-        "three-a", "three-c", "two-a", "two-b", "one-a", "one-c"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.4a>
     })
 
 test:do_test(
     "1.4b",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY +title DESC, +tn
         ]]
     end, {
         -- <1.4b>
-        "three-a", "three-c", "two-a", "two-b", "one-a", "one-c"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.4b>
     })
 
@@ -180,13 +187,13 @@ test:do_test(
 test:do_test(
     "1.4c",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             EXPLAIN QUERY PLAN
             SELECT name FROM album JOIN track USING (aid) ORDER BY title DESC, tn
         ]]
     end, {
         -- <1.4c>
-        "~/ORDER BY/"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.4c>
     })
 
@@ -194,24 +201,24 @@ test:do_test(
 test:do_test(
     "1.5a",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY title, tn DESC
         ]]
     end, {
         -- <1.5a>
-        "one-c", "one-a", "two-b", "two-a", "three-c", "three-a"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.5a>
     })
 
 test:do_test(
     "1.5b",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY +title, +tn DESC
         ]]
     end, {
         -- <1.5b>
-        "one-c", "one-a", "two-b", "two-a", "three-c", "three-a"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.5b>
     })
 
@@ -219,13 +226,13 @@ test:do_test(
 test:do_test(
     "1.5c",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             EXPLAIN QUERY PLAN
             SELECT name FROM album JOIN track USING (aid) ORDER BY title, tn DESC
         ]]
     end, {
         -- <1.5c>
-        "~/ORDER BY/"
+        1, "ORDER BY does not support different sorting orders"
         -- </1.5c>
     })
 
@@ -447,12 +454,12 @@ test:do_test(
 test:do_test(
     "3.1a",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album CROSS JOIN track USING (aid) ORDER BY title, tn DESC
         ]]
     end, {
         -- <3.1a>
-        "one-c", "one-a", "two-b", "two-a", "three-c", "three-a"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.1a>
     })
 
@@ -461,13 +468,13 @@ test:do_test(
 test:do_test(
     "3.1b",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             EXPLAIN QUERY PLAN
             SELECT name FROM album CROSS JOIN track USING (aid) ORDER BY title, tn DESC
         ]]
     end, {
         -- <3.1b>
-        "~/ORDER BY/"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.1b>
     })
 
@@ -478,12 +485,12 @@ test:do_test(
 test:do_test(
     "3.2a",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY +title, +tn DESC
         ]]
     end, {
         -- <3.2a>
-        "one-c", "one-a", "two-b", "two-a", "three-c", "three-a"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.2a>
     })
 
@@ -492,13 +499,13 @@ test:do_test(
 test:do_test(
     "3.2b",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             EXPLAIN QUERY PLAN
             SELECT name FROM album JOIN track USING (aid) ORDER BY +title, +tn DESC
         ]]
     end, {
         -- <3.2b>
-        "/ORDER BY/"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.2b>
     })
 
@@ -510,12 +517,12 @@ test:do_test(
     function()
         -- X(374, "X!cmd", [=[["optimization_control","db","order-by-idx-join","0"]]=])
         -- db("cache", "flush")
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album JOIN track USING (aid) ORDER BY title, tn DESC
         ]]
     end, {
         -- <3.3a>
-        "one-c", "one-a", "two-b", "two-a", "three-c", "three-a"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.3a>
     })
 
@@ -620,25 +627,25 @@ test:do_test(
 test:do_test(
     "3.6a",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album CROSS JOIN track USING (aid) ORDER BY title DESC, tn
         ]]
     end, {
         -- <3.6a>
-        "three-a", "three-c", "two-a", "two-b", "one-a", "one-c"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.6a>
     })
 
 test:do_test(
     "3.6b",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT name FROM album CROSS JOIN track USING (aid)
              ORDER BY +title DESC, +tn
         ]]
     end, {
         -- <3.6b>
-        "three-a", "three-c", "two-a", "two-b", "one-a", "one-c"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.6b>
     })
 
@@ -646,13 +653,13 @@ test:do_test(
 test:do_test(
     "3.6c",
     function()
-        return test:execsql [[
+        return test:catchsql [[
             EXPLAIN QUERY PLAN
             SELECT name FROM album CROSS JOIN track USING (aid) ORDER BY title DESC, tn
         ]]
     end, {
         -- <3.6c>
-        "~/ORDER BY/"
+        1, "ORDER BY does not support different sorting orders"
         -- </3.6c>
     })
 
@@ -725,7 +732,7 @@ test:do_execsql_test(
 -- verifies that a PseudoTable cursor is not closed prematurely in a deeply
 -- nested query.  This test caused a segfault on 3.8.5 beta.
 --
-test:do_execsql_test(
+test:do_catchsql_test(
     6.0,
     [[
         CREATE TABLE abc(a INT primary key, b INT, c INT);
@@ -740,7 +747,7 @@ test:do_execsql_test(
         FROM abc;
     ]], {
         -- <6.0>
-        "hardware", "hardware", "hardware"
+        1, "ORDER BY does not support different sorting orders"
         -- </6.0>
     })
 
diff --git a/test/sql-tap/orderby2.test.lua b/test/sql-tap/orderby2.test.lua
index 3cee73f25..f184e0f47 100755
--- a/test/sql-tap/orderby2.test.lua
+++ b/test/sql-tap/orderby2.test.lua
@@ -17,6 +17,13 @@ test:plan(9)
 -- focus of this file is testing that the optimizations that disable
 -- ORDER BY clauses when the natural order of a query is correct.
 --
+-- NOTE: some tests in this file and also in another orderby test
+-- files are expected to fail with "ORDER BY does not support
+-- different sorting orders" error. This behavior is temporary
+-- and corresponding tests must be fixed when different sorting
+-- orders will be allowed in ORDER BY. For details please see
+-- tickets #4038 and #3309.
+--
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "orderby2"
@@ -118,7 +125,7 @@ test:do_test(
 test:do_test(
     2.0,
     function()
-        return test:execsql [[
+        return test:catchsql [[
             CREATE TABLE t31(a INT ,b INT , PRIMARY KEY(a,b));
             CREATE TABLE t32(c INT ,d INT , PRIMARY KEY(c,d));
             CREATE TABLE t33(e INT ,f INT , PRIMARY KEY(e,f));
@@ -135,21 +142,21 @@ test:do_test(
         ]]
     end, {
         -- <2.0>
-        "1,3,7,10", "1,3,7,14", "1,3,6,11", "1,4,8,12", "1,4,8,12", "1,4,8,13", "1,4,5,9", "2,3,7,10", "2,3,7,14", "2,3,6,11"
+        1, "ORDER BY does not support different sorting orders",
         -- </2.0>
     })
 
 test:do_test(
     2.1,
     function()
-        return test:execsql [[
+        return test:catchsql [[
             SELECT CAST(a AS TEXT)||','||CAST(c AS TEXT)||','||CAST(e AS TEXT)||','||CAST(g AS TEXT) FROM t31, t32, t33, t34
              WHERE c=b AND e=d AND g=f
              ORDER BY +a ASC, +c ASC, +e DESC, +g ASC;
         ]]
     end, {
         -- <2.1>
-        "1,3,7,10", "1,3,7,14", "1,3,6,11", "1,4,8,12", "1,4,8,12", "1,4,8,13", "1,4,5,9", "2,3,7,10", "2,3,7,14", "2,3,6,11"
+        1, "ORDER BY does not support different sorting orders",
         -- </2.1>
     })
 
diff --git a/test/sql-tap/orderby6.test.lua b/test/sql-tap/orderby6.test.lua
index 213e87421..ce807a503 100755
--- a/test/sql-tap/orderby6.test.lua
+++ b/test/sql-tap/orderby6.test.lua
@@ -16,6 +16,13 @@ test:plan(52)
 -- This file implements regression tests for sql library.  The
 -- focus of this file is testing that the block-sort optimization.
 --
+-- NOTE: some tests in this file and also in another orderby test
+-- files are expected to fail with "ORDER BY does not support
+-- different sorting orders" error. This behavior is temporary
+-- and corresponding tests must be fixed when different sorting
+-- orders will be allowed in ORDER BY. For details please see
+-- tickets #4038 and #3309.
+--
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "orderby6"
@@ -54,28 +61,31 @@ testprefix = "orderby6"
         test:execsql "SELECT b,a,c FROM t1 ORDER BY +b,+a,+c"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.3",
         [[
             SELECT b,a,c FROM t1 ORDER BY b,c DESC,a;
-        ]], 
-        test:execsql "SELECT b,a,c FROM t1 ORDER BY +b,+c DESC,+a"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT b,a,c FROM t1 ORDER BY +b,+c DESC,+a"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.4",
         [[
             SELECT b,a,c FROM t1 ORDER BY b DESC,c,a;
-        ]], 
-        test:execsql "SELECT b,a,c FROM t1 ORDER BY +b DESC,+c,+a"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT b,a,c FROM t1 ORDER BY +b DESC,+c,+a"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.5",
         [[
             SELECT b,a,c FROM t1 ORDER BY b DESC,a,c;
-        ]], 
-        test:execsql "SELECT b,a,c FROM t1 ORDER BY +b DESC,+a,+c"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT b,a,c FROM t1 ORDER BY +b DESC,+a,+c"
         )
 
     -- LIMIT and OFFSET clauses on block-sort queries.
@@ -150,6 +160,10 @@ testprefix = "orderby6"
     -- Many test cases where the LIMIT+OFFSET window is in various
     -- alignments with block-sort boundaries.
     --
+    -- Test cases with 'err=true' are expected to fail,
+    -- if different sorting orders are not supported in
+    -- ORDER BY clause.
+    --
     local data = {
         {limit=0, offset=4, orderby="+b,+a"},
         {limit=0, offset=5, orderby="+b,+a"},
@@ -159,22 +173,22 @@ testprefix = "orderby6"
         {limit=0, offset=1, orderby="+b,+a"},
         {limit=7, offset=4, orderby="+b,+a"},
         {limit=7, offset=9, orderby="+b,+a"},
-        {limit=0, offset=4, orderby="+b DESC,+a"},
-        {limit=0, offset=5, orderby="+b DESC,+a"},
-        {limit=0, offset=6, orderby="+b DESC,+a"},
-        {limit=0, offset=9, orderby="+b DESC,+a"},
-        {limit=0, offset=0, orderby="+b DESC,+a"},
-        {limit=0, offset=1, orderby="+b DESC,+a"},
-        {limit=7, offset=4, orderby="+b DESC,+a"},
-        {limit=7, offset=9, orderby="+b DESC,+a"},
-        {limit=0, offset=4, orderby="+b,+a DESC"},
-        {limit=0, offset=5, orderby="+b,+a DESC"},
-        {limit=0, offset=6, orderby="+b,+a DESC"},
-        {limit=0, offset=9, orderby="+b,+a DESC"},
-        {limit=0, offset=0, orderby="+b,+a DESC"},
-        {limit=0, offset=1, orderby="+b,+a DESC"},
-        {limit=7, offset=4, orderby="+b,+a DESC"},
-        {limit=7, offset=9, orderby="+b,+a DESC"},
+        {limit=0, offset=4, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=5, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=6, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=9, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=0, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=1, orderby="+b DESC,+a", err=true},
+        {limit=7, offset=4, orderby="+b DESC,+a", err=true},
+        {limit=7, offset=9, orderby="+b DESC,+a", err=true},
+        {limit=0, offset=4, orderby="+b,+a DESC", err=true},
+        {limit=0, offset=5, orderby="+b,+a DESC", err=true},
+        {limit=0, offset=6, orderby="+b,+a DESC", err=true},
+        {limit=0, offset=9, orderby="+b,+a DESC", err=true},
+        {limit=0, offset=0, orderby="+b,+a DESC", err=true},
+        {limit=0, offset=1, orderby="+b,+a DESC", err=true},
+        {limit=7, offset=4, orderby="+b,+a DESC", err=true},
+        {limit=7, offset=9, orderby="+b,+a DESC", err=true},
         {limit=0, offset=4, orderby="+b DESC,+a DESC"},
         {limit=0, offset=5, orderby="+b DESC,+a DESC"},
         {limit=0, offset=6, orderby="+b DESC,+a DESC"},
@@ -183,10 +197,16 @@ testprefix = "orderby6"
         {limit=0, offset=1, orderby="+b DESC,+a DESC"},
         {limit=7, offset=4, orderby="+b DESC,+a DESC"},
         {limit=7, offset=9, orderby="+b DESC,+a DESC"}}
+    local expected_err_res = {1, "ORDER BY does not support different sorting orders"}
     for i, v in ipairs(data) do
-        local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-        local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-        test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
+        if v.err then
+            local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+            test:do_catchsql_test("1.21."..i, sql1, expected_err_res)
+        else
+            local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+            local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+            test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
+        end
     end
 
 --     for _ in X(0, "X!foreach", [=[["tx limit offset orderby","\n     1  10 24 {+b,+a}\n     2  10 25 {+b,+a}\n     3  10 26 {+b,+a}\n     4  10 39 {+b,+a}\n     5  10 40 {+b,+a}\n     6  10 41 {+b,+a}\n     7  27 24 {+b,+a}\n     8  27 49 {+b,+a}\n     11 10 24 {+b DESC,+a}\n     12 10 25 {+b DESC,+a}\n     13 10 26 {+b DESC,+a}\n     14 10 39 {+b DESC,+a}\n     15 10 40 {+b DESC,+a}\n     16 10 41 {+b DESC,+a}\n     17 27 24 {+b DESC,+a}\n     18 27 49 {+b DESC,+a}\n     21 10 24 {+b,+a DESC}\n     22 10 25 {+b,+a DESC}\n     23 10 26 {+b,+a DESC}\n     24 10 39 {+b,+a DESC}\n     25 10 40 {+b,+a DESC}\n     26 10 41 {+b,+a DESC}\n     27 27 24 {+b,+a DESC}\n     28 27 49 {+b,+a DESC}\n     31 10 24 {+b DESC,+a DESC}\n     32 10 25 {+b DESC,+a DESC}\n     33 10 26 {+b DESC,+a DESC}\n     34 10 39 {+b DESC,+a DESC}\n     35 10 40 {+b DESC,+a DESC}\n     36 10 41 {+b DESC,+a DESC}\n     37 27 24 {+b DESC,+a DESC}\n     38 27 49 {+b DESC,+a DESC}\n  "]]=]) do
@@ -258,59 +278,66 @@ testprefix = "orderby6"
         test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.37",
         [[
             SELECT a FROM t2 ORDER BY b,c,d,e,f DESC;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC;"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.38",
         [[
             SELECT a FROM t2 ORDER BY b,c,d,e DESC,f;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e DESC,+f;"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e DESC,+f;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.39",
         [[
             SELECT a FROM t2 ORDER BY b,c,d DESC,e,f;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d DESC,+e,+f;"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d DESC,+e,+f;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.40",
         [[
             SELECT a FROM t2 ORDER BY b,c DESC,d,e,f;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c DESC,+d,+e,+f;"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b,+c DESC,+d,+e,+f;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.41",
         [[
             SELECT a FROM t2 ORDER BY b DESC,c,d,e,f;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c,+d,+e,+f;"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c,+d,+e,+f;"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.42",
         [[
             SELECT a FROM t2 ORDER BY b DESC,c DESC,d,e,f LIMIT 31;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c DESC,+d,+e,+f LIMIT 31"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c DESC,+d,+e,+f LIMIT 31"
         )
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1.43",
         [[
             SELECT a FROM t2 ORDER BY b,c,d,e,f DESC LIMIT 8 OFFSET 7;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC LIMIT 8 OFFSET 7"
+        ]],
+        {1, "ORDER BY does not support different sorting orders"}
+        --test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC LIMIT 8 OFFSET 7"
         )
 test:finish_test()
diff --git a/test/sql-tap/select1.test.lua b/test/sql-tap/select1.test.lua
index 6c35b6f95..4e0795109 100755
--- a/test/sql-tap/select1.test.lua
+++ b/test/sql-tap/select1.test.lua
@@ -855,34 +855,37 @@ test:do_catchsql_test(
     })
 
 -- MUST_WORK_TEST
-test:do_execsql_test(
+test:do_catchsql_test(
     "select1-4.11",
     [[
         INSERT INTO t5 VALUES(3,10);
         SELECT * FROM t5 ORDER BY 2, 1 DESC;
     ]], {
         -- <select1-4.11>
-        2, 9, 3, 10, 1, 10
+        1, "ORDER BY does not support different sorting orders"
+        --2, 9, 3, 10, 1, 10
         -- </select1-4.11>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "select1-4.12",
     [[
         SELECT * FROM t5 ORDER BY 1 DESC, b;
     ]], {
         -- <select1-4.12>
-        3, 10, 2, 9, 1, 10
+        1, "ORDER BY does not support different sorting orders"
+        --3, 10, 2, 9, 1, 10
         -- </select1-4.12>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "select1-4.13",
     [[
         SELECT * FROM t5 ORDER BY b DESC, 1;
     ]], {
         -- <select1-4.13>
-        1, 10, 3, 10, 2, 9
+        1, "ORDER BY does not support different sorting orders"
+        --1, 10, 3, 10, 2, 9
         -- </select1-4.13>
     })
 
diff --git a/test/sql-tap/select4.test.lua b/test/sql-tap/select4.test.lua
index bd2ada99c..72d349447 100755
--- a/test/sql-tap/select4.test.lua
+++ b/test/sql-tap/select4.test.lua
@@ -541,7 +541,8 @@ test:do_catchsql_test(
         ORDER BY 1, 2 DESC;
     ]], {
         -- <select4-5.2j>
-        0, {1, 5, 1, 4, 1, 3, 1, 2, 1, 1, 1, 0, 2, 8, 2, 7, 2, 6, 2, 5}
+        1, "ORDER BY does not support different sorting orders"
+        --0, {1, 5, 1, 4, 1, 3, 1, 2, 1, 1, 1, 0, 2, 8, 2, 7, 2, 6, 2, 5}
         -- </select4-5.2j>
     })
 
diff --git a/test/sql-tap/sort.test.lua b/test/sql-tap/sort.test.lua
index a84c549cc..4df17d249 100755
--- a/test/sql-tap/sort.test.lua
+++ b/test/sql-tap/sort.test.lua
@@ -189,33 +189,36 @@ test:do_execsql_test(
         -- </sort-1.8.3>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-1.9",
     [[
         SELECT n FROM t1 ORDER BY log, flt DESC
     ]], {
         -- <sort-1.9>
-        1, 3, 2, 7, 6, 4, 5, 8
+        1, "ORDER BY does not support different sorting orders",
+        --1, 3, 2, 7, 6, 4, 5, 8
         -- </sort-1.9>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-1.9.1",
     [[
         SELECT n FROM t1 ORDER BY log ASC, flt DESC
     ]], {
         -- <sort-1.9.1>
-        1, 3, 2, 7, 6, 4, 5, 8
+        1, "ORDER BY does not support different sorting orders"
+        --1, 3, 2, 7, 6, 4, 5, 8
         -- </sort-1.9.1>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-1.10",
     [[
         SELECT n FROM t1 ORDER BY log DESC, flt
     ]], {
         -- <sort-1.10>
-        8, 5, 4, 6, 7, 2, 3, 1
+        1, "ORDER BY does not support different sorting orders"
+        --8, 5, 4, 6, 7, 2, 3, 1
         -- </sort-1.10>
     })
 
@@ -427,23 +430,25 @@ test:do_execsql_test(
         -- </sort-5.1>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-5.2",
     [[
         select a from t3 order by b, a desc;
     ]], {
         -- <sort-5.2>
-        6, 5, 3, 2, 1, 4
+        1, "ORDER BY does not support different sorting orders"
+        --6, 5, 3, 2, 1, 4
         -- </sort-5.2>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-5.3",
     [[
         select a from t3 order by b desc, a;
     ]], {
         -- <sort-5.3>
-        4, 1, 2, 3, 5, 6
+        1, "ORDER BY does not support different sorting orders"
+        --4, 1, 2, 3, 5, 6
         -- </sort-5.3>
     })
 
@@ -468,23 +473,25 @@ test:do_execsql_test(
         -- </sort-6.1>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-6.2",
     [[
         select a from t3 order by b, a desc;
     ]], {
         -- <sort-6.2>
-        6, 5, 3, 2, 1, 4
+        1, "ORDER BY does not support different sorting orders"
+        --6, 5, 3, 2, 1, 4
         -- </sort-6.2>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "sort-6.3",
     [[
         select a from t3 order by b desc, a;
     ]], {
         -- <sort-6.3>
-        4, 1, 2, 3, 5, 6
+        1, "ORDER BY does not support different sorting orders"
+        --4, 1, 2, 3, 5, 6
         -- </sort-6.3>
     })
 
diff --git a/test/sql-tap/tkt-4dd95f6943.test.lua b/test/sql-tap/tkt-4dd95f6943.test.lua
index 21d6a18ff..34a5b0fab 100755
--- a/test/sql-tap/tkt-4dd95f6943.test.lua
+++ b/test/sql-tap/tkt-4dd95f6943.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(319)
+test:plan(152)
 
 --!./tcltestrunner.lua
 -- 2013 March 13
@@ -18,6 +18,24 @@ test:plan(319)
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "tkt-4dd95f6943"
+
+-- NOTE: many tests in this file are commented, because
+-- different sort orders in ORDER BY clauses are TEMPORARY
+-- disabled because of the problem described in tickets
+-- #4038 and #3309. Please, uncomment these tests when different
+-- orders will be allowed again. If you see that test below
+-- starts failing, then it is probably the time to uncomment
+-- tests.
+test:do_catchsql_test(
+    0.0,
+    [[
+        CREATE TABLE t0(id INT primary key, x INT);
+        INSERT INTO t0 VALUES(1,3), (3,1);
+        SELECT * FROM t0 ORDER BY id DESC, x;
+    ]],
+    {1, "ORDER BY does not support different sorting orders"}
+)
+
 test:do_execsql_test(
     1.0,
     [[
@@ -113,21 +131,21 @@ for tn1, idx in ipairs(indexes) do
                 1, 2, 1, 4, 1, 5
             })
 
-        test:do_execsql_test(
-            string.format("2.%s.%s.2", tn1, tn2),
-            string.format([[
-                SELECT x, y FROM t2 WHERE x = 2 AND y IN %s ORDER BY x ASC, y DESC;
-            ]], inexpr), {
-                2, 5, 2, 4, 2, 2
-            })
+        --test:do_execsql_test(
+        --    string.format("2.%s.%s.2", tn1, tn2),
+        --    string.format([[
+        --        SELECT x, y FROM t2 WHERE x = 2 AND y IN %s ORDER BY x ASC, y DESC;
+        --    ]], inexpr), {
+        --        2, 5, 2, 4, 2, 2
+        --    })
 
-        test:do_execsql_test(
-            string.format("2.%s.%s.3", tn1, tn2),
-            string.format([[
-                SELECT x, y FROM t2 WHERE x = 3 AND y IN %s ORDER BY x DESC, y ASC;
-            ]], inexpr), {
-                3, 2, 3, 4, 3, 5
-            })
+        --test:do_execsql_test(
+        --    string.format("2.%s.%s.3", tn1, tn2),
+        --    string.format([[
+        --        SELECT x, y FROM t2 WHERE x = 3 AND y IN %s ORDER BY x DESC, y ASC;
+        --    ]], inexpr), {
+        --        3, 2, 3, 4, 3, 5
+        --    })
 
         test:do_execsql_test(
             string.format("2.%s.%s.4", tn1, tn2),
@@ -155,50 +173,50 @@ for tn1, idx in ipairs(indexes) do
                 2, 1, 2, 2, 1, 4, 2, 1, 5
             })
 
-        test:do_execsql_test(
-            string.format("2.%s.%s.7", tn1, tn2),
-            string.format([[
-                SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s 
-                ORDER BY a, x ASC, y DESC;
-            ]], inexpr), {
-                4, 1, 5, 4, 1, 4, 4, 1, 2
-            })
+        --test:do_execsql_test(
+        --    string.format("2.%s.%s.7", tn1, tn2),
+        --    string.format([[
+        --        SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s
+        --        ORDER BY a, x ASC, y DESC;
+        --    ]], inexpr), {
+        --        4, 1, 5, 4, 1, 4, 4, 1, 2
+        --    })
 
-        test:do_execsql_test(
-            "2."..tn1..".8",
-            string.format([[
-                SELECT a, x, y FROM t2, t3 WHERE a = 2 AND x = 1 AND y IN %s 
-                ORDER BY x ASC, y DESC;
-            ]], inexpr), {
-                2, 1, 5, 2, 1, 4, 2, 1, 2
-            })
+        --test:do_execsql_test(
+        --    "2."..tn1..".8",
+        --    string.format([[
+        --        SELECT a, x, y FROM t2, t3 WHERE a = 2 AND x = 1 AND y IN %s
+        --        ORDER BY x ASC, y DESC;
+        --    ]], inexpr), {
+        --        2, 1, 5, 2, 1, 4, 2, 1, 2
+        --    })
 
-        test:do_execsql_test(
-            string.format("2.%s.%s.9", tn1, tn2),
-            string.format([[
-                SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s 
-                ORDER BY a, x DESC, y ASC;
-            ]], inexpr), {
-                4, 1, 2, 4, 1, 4, 4, 1, 5
-            })
+        --test:do_execsql_test(
+        --    string.format("2.%s.%s.9", tn1, tn2),
+        --    string.format([[
+        --        SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s
+        --        ORDER BY a, x DESC, y ASC;
+        --    ]], inexpr), {
+        --        4, 1, 2, 4, 1, 4, 4, 1, 5
+        --    })
 
-        test:do_execsql_test(
-            "2."..tn1..".10",
-            string.format([[
-                SELECT a, x, y FROM t2, t3 WHERE a = 2 AND x = 1 AND y IN %s 
-                ORDER BY x DESC, y ASC;
-            ]], inexpr), {
-                2, 1, 2, 2, 1, 4, 2, 1, 5
-            })
+        --test:do_execsql_test(
+        --    "2."..tn1..".10",
+        --    string.format([[
+        --        SELECT a, x, y FROM t2, t3 WHERE a = 2 AND x = 1 AND y IN %s
+        --        ORDER BY x DESC, y ASC;
+        --    ]], inexpr), {
+        --        2, 1, 2, 2, 1, 4, 2, 1, 5
+        --    })
 
-        test:do_execsql_test(
-            string.format("2.%s.%s.11", tn1, tn2),
-            string.format([[
-                SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s 
-                ORDER BY a, x DESC, y DESC;
-            ]], inexpr), {
-                4, 1, 5, 4, 1, 4, 4, 1, 2
-            })
+        --test:do_execsql_test(
+        --    string.format("2.%s.%s.11", tn1, tn2),
+        --    string.format([[
+        --        SELECT a, x, y FROM t2, t3 WHERE a = 4 AND x = 1 AND y IN %s
+        --        ORDER BY a, x DESC, y DESC;
+        --    ]], inexpr), {
+        --        4, 1, 5, 4, 1, 4, 4, 1, 2
+        --    })
 
         test:do_execsql_test(
             string.format("2.%s.%s.12", tn1, tn2),
diff --git a/test/sql-tap/tkt-b75a9ca6b0.test.lua b/test/sql-tap/tkt-b75a9ca6b0.test.lua
index ea684a73d..ebca47e72 100755
--- a/test/sql-tap/tkt-b75a9ca6b0.test.lua
+++ b/test/sql-tap/tkt-b75a9ca6b0.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(22)
+test:plan(21)
 
 --!./tcltestrunner.lua
 -- 2014-04-21
@@ -44,32 +44,35 @@ local tblscan = {0, 0, 0, "SCAN TABLE T1"}
 local grpsort = {0, 0, 0, "USE TEMP B-TREE FOR GROUP BY"}
 local sort = {0, 0, 0, "USE TEMP B-TREE FOR ORDER BY"}
 local eqps = {
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x,y", {1, 3,  2, 2,  3, 1}, {idxscan}},
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x", {1, 3, 2, 2, 3, 1}, {idxscan, sort}},
-    {"SELECT x,y FROM t1 GROUP BY y, x ORDER BY y, x", {3, 1, 2, 2, 1, 3}, {idxscan, sort}},
-    {"SELECT x,y FROM t1 GROUP BY x ORDER BY x", {1, 3, 2, 2, 3, 1}, {idxscan}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x,y", {0, {1, 3,  2, 2,  3, 1}}, {idxscan}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x", {0, {1, 3, 2, 2, 3, 1}}, {idxscan, sort}},
+    {"SELECT x,y FROM t1 GROUP BY y, x ORDER BY y, x", {0, {3, 1, 2, 2, 1, 3}}, {idxscan, sort}},
+    {"SELECT x,y FROM t1 GROUP BY x ORDER BY x", {0, {1, 3, 2, 2, 3, 1}}, {idxscan}},
     -- idxscan->tblscan after reorderind indexes list
     -- but it does not matter
-    {"SELECT x,y FROM t1 GROUP BY y ORDER BY y", {3, 1, 2, 2, 1, 3}, {tblscan, grpsort}},
+    {"SELECT x,y FROM t1 GROUP BY y ORDER BY y", {0, {3, 1, 2, 2, 1, 3}}, {tblscan, grpsort}},
     -- idxscan->tblscan after reorderind indexes list
     -- but it does not matter (because it does full scan)
-    {"SELECT x,y FROM t1 GROUP BY y ORDER BY x", {1, 3, 2, 2, 3, 1}, {tblscan, grpsort, sort}},
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x, y DESC", {1, 3, 2, 2, 3, 1}, {idxscan, sort}},
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x DESC, y DESC", {3, 1, 2, 2, 1, 3}, {idxscan, sort}},
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x ASC, y ASC", {1, 3, 2, 2, 3, 1}, {idxscan}},
-    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x COLLATE \"unicode_ci\", y", {1, 3, 2, 2, 3, 1}, {idxscan, sort}},
+    {"SELECT x,y FROM t1 GROUP BY y ORDER BY x", {0, {1, 3, 2, 2, 3, 1}}, {tblscan, grpsort, sort}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x, y DESC", {1, "ORDER BY does not support different sorting orders"}, {idxscan, sort}},
+    --{"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x, y DESC", {0, {1, 3, 2, 2, 3, 1}}, {idxscan, sort}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x DESC, y DESC", {0, {3, 1, 2, 2, 1, 3}}, {idxscan, sort}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x ASC, y ASC", {0, {1, 3, 2, 2, 3, 1}}, {idxscan}},
+    {"SELECT x,y FROM t1 GROUP BY x, y ORDER BY x COLLATE \"unicode_ci\", y", {0, {1, 3, 2, 2, 3, 1}}, {idxscan, sort}},
 }
 for tn, val in ipairs(eqps) do
     local q = val[1]
     local res = val[2]
     local eqp = val[3]
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "1."..tn..".1",
         q, res)
 
-    test:do_eqp_test(
-        "1."..tn..".2",
-        q, eqp)
+    if res[1] == 0 then
+        test:do_eqp_test(
+            "1."..tn..".2",
+            q, eqp)
+    end
 
 end
 test:finish_test()
diff --git a/test/sql-tap/tkt-ba7cbfaedc.test.lua b/test/sql-tap/tkt-ba7cbfaedc.test.lua
index 2aad10f2d..77692adec 100755
--- a/test/sql-tap/tkt-ba7cbfaedc.test.lua
+++ b/test/sql-tap/tkt-ba7cbfaedc.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(19)
+test:plan(12)
 
 --!./tcltestrunner.lua
 -- 2014-10-11
@@ -19,6 +19,24 @@ test:plan(19)
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "tkt-ba7cbfaedc"
+
+-- NOTE: some tests in this file are commented, because
+-- different sort orders in ORDER BY clauses are TEMPORARY
+-- disabled because of the problem described in tickets
+-- #4038 and #3309. Please, uncomment these tests when different
+-- orders will be allowed again. If you see that test below
+-- starts failing, then it is probably the time to uncomment
+-- tests.
+test:do_catchsql_test(
+    0.0,
+    [[
+        CREATE TABLE t0(id INT primary key, x INT);
+        INSERT INTO t0 VALUES(1,3), (3,1);
+        SELECT * FROM t0 ORDER BY id DESC, x;
+    ]],
+    {1, "ORDER BY does not support different sorting orders"}
+)
+
 test:do_execsql_test(
     1,
     [[
@@ -49,8 +67,8 @@ for n, idx in ipairs(idxs) do
     test:execsql(idx)
     local queries = {
         {"GROUP BY x, y ORDER BY x, y", {1, 'a', 1, "b", 2, "a", 2, "b", 3, "a", 3, "b"}},
-        {"GROUP BY x, y ORDER BY x DESC, y", {3, "a", 3, "b", 2, "a", 2, "b", 1, "a", 1, "b"}},
-        {"GROUP BY x, y ORDER BY x, y DESC", {1, "b", 1, "a", 2, "b", 2, "a", 3, "b", 3, "a"}},
+        --{"GROUP BY x, y ORDER BY x DESC, y", {3, "a", 3, "b", 2, "a", 2, "b", 1, "a", 1, "b"}},
+        --{"GROUP BY x, y ORDER BY x, y DESC", {1, "b", 1, "a", 2, "b", 2, "a", 3, "b", 3, "a"}},
         {"GROUP BY x, y ORDER BY x DESC, y DESC", {3, "b", 3, "a", 2, "b", 2, "a", 1, "b", 1, "a"}},
     }
     for tn, val in ipairs(queries) do
diff --git a/test/sql-tap/where2.test.lua b/test/sql-tap/where2.test.lua
index 8eaf4053d..9d931b53e 100755
--- a/test/sql-tap/where2.test.lua
+++ b/test/sql-tap/where2.test.lua
@@ -402,18 +402,26 @@ test:do_test(
         -- </where2-4.6c>
     })
 
-test:do_test(
+test:do_catchsql_test(
     "where2-4.6d",
-    function()
-        return queryplan([[
+    [[
       SELECT w,x,y,z FROM t1
        WHERE x IN (1,2,3,4,5,6,7,8)
          AND y IN (10000,10001,10002,10003,10004,10005)
        ORDER BY x, y DESC
-    ]])
-    end, {
+    ]],
+    --function()
+    --    return queryplan([[
+    --  SELECT w,x,y,z FROM t1
+    --   WHERE x IN (1,2,3,4,5,6,7,8)
+    --     AND y IN (10000,10001,10002,10003,10004,10005)
+    --   ORDER BY x, y DESC
+    --]])
+    --end,
+    {
         -- <where2-4.6d>
-        99, 6, 10000, 10006, "sort", "T1", "I1XY"
+        1, "ORDER BY does not support different sorting orders"
+        --99, 6, 10000, 10006, "sort", "T1", "I1XY"
         -- </where2-4.6d>
     })
 
-- 
2.20.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-11 15:44 [tarantool-patches] [PATCH] sql: forbid different sorting orders in ORDER BY Ivan Koptelov
@ 2019-03-12 14:25 ` n.pettik
  2019-03-15 12:10   ` i.koptelov
  2019-03-19 11:25 ` Kirill Yukhin
  1 sibling, 1 reply; 7+ messages in thread
From: n.pettik @ 2019-03-12 14:25 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Ivan Koptelov



> On 11 Mar 2019, at 18:44, Ivan Koptelov <ivan.koptelov@tarantool.org> wrote:
> 
> Without multi-directional iterators (to be introduced in #3309)
> in most cases (except those when internal VDBE merge sorter is
> used) ORDER BY with different specified ordered leads to wrong
> result. So for now (till #3309 is resolved) different sort
> orders are forbidden.
> 
> Close #4038
> 
> @TarantoolBot document
> Title: different sorting orders in ORDER BY are forbidden now
> The error will be raised if different sorting orders are
> encountered.

Since different sorting orders still can be accepted
(for instance, if you remove LIMIT clause from query mentioned
in issue description, it will work fine), mb it is worth to abort
execution only when it comes to sorting by ephemeral space.
Please, ask other members of server team for their opinion
on this case. Pros: different sorting orders will be available
in most cases; cons: this feature will be inconsistent - in some
cases it will work, for other queries it will throw an error.

> ---
> Branch https://github.com/tarantool/tarantool/tree/sudobobo/gh-4038-ban-diff-orders-in-ORDER-BY
> Issue https://github.com/tarantool/tarantool/issues/4038
> 
> src/box/sql/expr.c                   |  15 +++
> src/box/sql/parse.y                  |   5 +-
> src/box/sql/sqlInt.h                 |  14 +++
> test/sql-tap/e_select1.test.lua      | 165 ++++++++++++++++++---------
> test/sql-tap/orderby1.test.lua       |  67 ++++++-----
> test/sql-tap/orderby2.test.lua       |  15 ++-
> test/sql-tap/orderby6.test.lua       | 125 ++++++++++++--------
> test/sql-tap/select1.test.lua        |  15 ++-
> test/sql-tap/select4.test.lua        |   3 +-
> test/sql-tap/sort.test.lua           |  35 +++---
> test/sql-tap/tkt-4dd95f6943.test.lua | 128 ++++++++++++---------
> test/sql-tap/tkt-b75a9ca6b0.test.lua |  33 +++---
> test/sql-tap/tkt-ba7cbfaedc.test.lua |  24 +++-
> test/sql-tap/where2.test.lua         |  20 +++-

Plase, make up a list of tests to be enabled after multi-directional
iterators are introduced. Don’t fix tests which fail now, simply comment
them. Finally, add a few simple tests verifying that different sorting
orders are not supported now.

A bit of code-style refactoring:

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 85719e3a1..bd494ef2e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1810,12 +1810,15 @@ sqlExprListSetSortOrder(struct ExprList *p, enum sort_order sort_order)
 }
 
 void
-sql_check_sort_orders(ExprList * expr_list, Parse *parse) {
+sql_expr_check_sort_orders(struct Parse *parse,
+                          const struct ExprList *expr_list)
+{
        if(expr_list == NULL)
                return;
-       for (int i = 0; i < expr_list->nExpr; i++) {
+       enum sort_order reference_order = expr_list->a[0].sort_order;
+       for (int i = 1; i < expr_list->nExpr; i++) {
                assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
-               if (expr_list->a[i].sort_order != expr_list->a[0].sort_order) {
+               if (expr_list->a[i].sort_order != reference_order) {
                        diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
                                 "different sorting orders");
                        sql_parser_error(parse);
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 95d6962f9..f056a89b4 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -662,7 +662,7 @@ using_opt(U) ::= .                        {U = 0;}
 
 orderby_opt(A) ::= .                          {A = 0;}
 orderby_opt(A) ::= ORDER BY sortlist(X). {
-  sql_check_sort_orders(X, pParse);
+  sql_check_sort_orders(pParse, X);
   A = X;
 }
 sortlist(A) ::= sortlist(A) COMMA expr(Y) sortorder(Z). {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 06818d9e1..324ab1618 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3238,18 +3238,19 @@ ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *);
 void sqlExprListSetSortOrder(ExprList *, enum sort_order sort_order);
 
 /**
- * Check if sorting orders are the same in ORDER BY and rise an
+ * Check if sorting orders are the same in ORDER BY and raise an
  * error if they are not.
  *
  * In future, we will support different sorting orders in
  * ORDER BY (e.g. ORDER BY col1 ASC, col2 DESC) and remove this
- * check (see ticket #3309).
- * @param expr_list Expression list with  ORDER BY clause
+ * check.
+ *  * @param parse Parsing context.
+ * @param expr_list Expression list with ORDER BY clause
  * at the end.
- * @param parse Parsing context.
  */
 void
-sql_check_sort_orders(ExprList * expr_list, Parse *parse);
+sql_expr_check_sort_orders(struct Parse *parse,
+                          const struct ExprList *expr_list);
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-12 14:25 ` [tarantool-patches] " n.pettik
@ 2019-03-15 12:10   ` i.koptelov
  2019-03-15 16:23     ` n.pettik
  0 siblings, 1 reply; 7+ messages in thread
From: i.koptelov @ 2019-03-15 12:10 UTC (permalink / raw)
  To: tarantool-patches; +Cc: n.pettik

Thank you for the review.

> On 12 Mar 2019, at 17:25, n.pettik <korablev@tarantool.org> wrote:
> 
>> 
>> On 11 Mar 2019, at 18:44, Ivan Koptelov <ivan.koptelov@tarantool.org> wrote:
>> 
>> Without multi-directional iterators (to be introduced in #3309)
>> in most cases (except those when internal VDBE merge sorter is
>> used) ORDER BY with different specified ordered leads to wrong
>> result. So for now (till #3309 is resolved) different sort
>> orders are forbidden.
>> 
>> Close #4038
>> 
>> @TarantoolBot document
>> Title: different sorting orders in ORDER BY are forbidden now
>> The error will be raised if different sorting orders are
>> encountered.
> 
> Since different sorting orders still can be accepted
> (for instance, if you remove LIMIT clause from query mentioned
> in issue description, it will work fine), mb it is worth to abort
> execution only when it comes to sorting by ephemeral space.
> Please, ask other members of server team for their opinion
> on this case. Pros: different sorting orders will be available
> in most cases; cons: this feature will be inconsistent - in some
> cases it will work, for other queries it will throw an error.
After a short discussion we decided to forbid only ORDER BY + LIMIT
+ (ASC + DESC). Here is corresponding code changes:

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 996f55d37..e2622222e 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -458,6 +458,8 @@ oneselect(A) ::= SELECT(S) distinct(D) selcollist(W) from(X) where_opt(Y)
 #ifdef SQL_DEBUG
   Token s = S; /*A-overwrites-S*/
 #endif
+  if (L.pLimit != NULL)
+    sql_expr_check_sort_orders(pParse, Z);
   A = sqlSelectNew(pParse,W,X,Y,P,Q,Z,D,L.pLimit,L.pOffset);

Commit message is also fixed.
> 
>> ---
>> Branch https://github.com/tarantool/tarantool/tree/sudobobo/gh-4038-ban-diff-orders-in-ORDER-BY
>> Issue https://github.com/tarantool/tarantool/issues/4038
>> 
>> src/box/sql/expr.c                   |  15 +++
>> src/box/sql/parse.y                  |   5 +-
>> src/box/sql/sqlInt.h                 |  14 +++
>> test/sql-tap/e_select1.test.lua      | 165 ++++++++++++++++++---------
>> test/sql-tap/orderby1.test.lua       |  67 ++++++-----
>> test/sql-tap/orderby2.test.lua       |  15 ++-
>> test/sql-tap/orderby6.test.lua       | 125 ++++++++++++--------
>> test/sql-tap/select1.test.lua        |  15 ++-
>> test/sql-tap/select4.test.lua        |   3 +-
>> test/sql-tap/sort.test.lua           |  35 +++---
>> test/sql-tap/tkt-4dd95f6943.test.lua | 128 ++++++++++++---------
>> test/sql-tap/tkt-b75a9ca6b0.test.lua |  33 +++---
>> test/sql-tap/tkt-ba7cbfaedc.test.lua |  24 +++-
>> test/sql-tap/where2.test.lua         |  20 +++-
> 
> Plase, make up a list of tests to be enabled after multi-directional
> iterators are introduced. Don’t fix tests which fail now, simply comment
> them. Finally, add a few simple tests verifying that different sorting
> orders are not supported now.

I did as you say. Added list of tests as a comment to #3309.
But I would like to know, why commenting tests & making a list
of them is better than change tests & comment old expected results?
The main advantage is that one can not ignore/miss tests, which
would fail after #3309 is done.

diff --git a/test/sql-tap/orderby6.test.lua b/test/sql-tap/orderby6.test.lua
index 213e87421..7fe0032dd 100755
--- a/test/sql-tap/orderby6.test.lua
+++ b/test/sql-tap/orderby6.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(52)
+test:plan(18)
 
 --!./tcltestrunner.lua
 -- 2014-03-21
@@ -16,6 +16,14 @@ test:plan(52)
 -- This file implements regression tests for sql library.  The
 -- focus of this file is testing that the block-sort optimization.
 --
+-- NOTE: tests with ORDER BY + LIMIT + (ASC + DESC, i.e
+-- different sorting orders) are commented in this file as they
+-- are failing because we don't have multi-directional iterators.
+-- Details and full list of commented tests can be found here:
+-- https://github.com/tarantool/tarantool/issues/3309
+-- Please, uncomment the tests when multi-directional iterators
+-- are introduced.
+--
 -- ["set","testdir",[["file","dirname",["argv0"]]]]
 -- ["source",[["testdir"],"\/tester.tcl"]]
 testprefix = "orderby6"
@@ -150,44 +158,48 @@ testprefix = "orderby6"
     -- Many test cases where the LIMIT+OFFSET window is in various
     -- alignments with block-sort boundaries.
     --
-    local data = {
-        {limit=0, offset=4, orderby="+b,+a"},
-        {limit=0, offset=5, orderby="+b,+a"},
-        {limit=0, offset=6, orderby="+b,+a"},
-        {limit=0, offset=9, orderby="+b,+a"},
-        {limit=0, offset=0, orderby="+b,+a"},
-        {limit=0, offset=1, orderby="+b,+a"},
-        {limit=7, offset=4, orderby="+b,+a"},
-        {limit=7, offset=9, orderby="+b,+a"},
-        {limit=0, offset=4, orderby="+b DESC,+a"},
-        {limit=0, offset=5, orderby="+b DESC,+a"},
-        {limit=0, offset=6, orderby="+b DESC,+a"},
-        {limit=0, offset=9, orderby="+b DESC,+a"},
-        {limit=0, offset=0, orderby="+b DESC,+a"},
-        {limit=0, offset=1, orderby="+b DESC,+a"},
-        {limit=7, offset=4, orderby="+b DESC,+a"},
-        {limit=7, offset=9, orderby="+b DESC,+a"},
-        {limit=0, offset=4, orderby="+b,+a DESC"},
-        {limit=0, offset=5, orderby="+b,+a DESC"},
-        {limit=0, offset=6, orderby="+b,+a DESC"},
-        {limit=0, offset=9, orderby="+b,+a DESC"},
-        {limit=0, offset=0, orderby="+b,+a DESC"},
-        {limit=0, offset=1, orderby="+b,+a DESC"},
-        {limit=7, offset=4, orderby="+b,+a DESC"},
-        {limit=7, offset=9, orderby="+b,+a DESC"},
-        {limit=0, offset=4, orderby="+b DESC,+a DESC"},
-        {limit=0, offset=5, orderby="+b DESC,+a DESC"},
-        {limit=0, offset=6, orderby="+b DESC,+a DESC"},
-        {limit=0, offset=9, orderby="+b DESC,+a DESC"},
-        {limit=0, offset=0, orderby="+b DESC,+a DESC"},
-        {limit=0, offset=1, orderby="+b DESC,+a DESC"},
-        {limit=7, offset=4, orderby="+b DESC,+a DESC"},
-        {limit=7, offset=9, orderby="+b DESC,+a DESC"}}
-    for i, v in ipairs(data) do
-        local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-        local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-        test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
-    end
+    -- Tests are commented because of the reason described in
+    -- NOTE at the beginning of the file.
+    -- <begin>
+    --local data = {
+    --    {limit=0, offset=4, orderby="+b,+a"},
+    --    {limit=0, offset=5, orderby="+b,+a"},
+    --    {limit=0, offset=6, orderby="+b,+a"},
+    --    {limit=0, offset=9, orderby="+b,+a"},
+    --    {limit=0, offset=0, orderby="+b,+a"},
+    --    {limit=0, offset=1, orderby="+b,+a"},
+    --    {limit=7, offset=4, orderby="+b,+a"},
+    --    {limit=7, offset=9, orderby="+b,+a"},
+    --    {limit=0, offset=4, orderby="+b DESC,+a"},
+    --    {limit=0, offset=5, orderby="+b DESC,+a"},
+    --    {limit=0, offset=6, orderby="+b DESC,+a"},
+    --    {limit=0, offset=9, orderby="+b DESC,+a"},
+    --    {limit=0, offset=0, orderby="+b DESC,+a"},
+    --    {limit=0, offset=1, orderby="+b DESC,+a"},
+    --    {limit=7, offset=4, orderby="+b DESC,+a"},
+    --    {limit=7, offset=9, orderby="+b DESC,+a"},
+    --    {limit=0, offset=4, orderby="+b,+a DESC"},
+    --    {limit=0, offset=5, orderby="+b,+a DESC"},
+    --    {limit=0, offset=6, orderby="+b,+a DESC"},
+    --    {limit=0, offset=9, orderby="+b,+a DESC"},
+    --    {limit=0, offset=0, orderby="+b,+a DESC"},
+    --    {limit=0, offset=1, orderby="+b,+a DESC"},
+    --    {limit=7, offset=4, orderby="+b,+a DESC"},
+    --    {limit=7, offset=9, orderby="+b,+a DESC"},
+    --    {limit=0, offset=4, orderby="+b DESC,+a DESC"},
+    --    {limit=0, offset=5, orderby="+b DESC,+a DESC"},
+    --    {limit=0, offset=6, orderby="+b DESC,+a DESC"},
+    --    {limit=0, offset=9, orderby="+b DESC,+a DESC"},
+    --    {limit=0, offset=0, orderby="+b DESC,+a DESC"},
+    --    {limit=0, offset=1, orderby="+b DESC,+a DESC"},
+    --    {limit=7, offset=4, orderby="+b DESC,+a DESC"},
+    --    {limit=7, offset=9, orderby="+b DESC,+a DESC"}}
+    --for i, v in ipairs(data) do
+    --    local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+    --    local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+    --    test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
+    --end
+    -- <end>
 
@@ -297,20 +309,23 @@ testprefix = "orderby6"
         ]], 
         test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c,+d,+e,+f;"
         )
-
-    test:do_execsql_test(
-        "1.42",
-        [[
-            SELECT a FROM t2 ORDER BY b DESC,c DESC,d,e,f LIMIT 31;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c DESC,+d,+e,+f LIMIT 31"
-        )
-
-    test:do_execsql_test(
-        "1.43",
-        [[
-            SELECT a FROM t2 ORDER BY b,c,d,e,f DESC LIMIT 8 OFFSET 7;
-        ]], 
-        test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC LIMIT 8 OFFSET 7"
-        )
+    -- Tests are commented because of the reason described in
+    -- NOTE at the beginning of the file.
+    -- <begin>
+    --test:do_execsql_test(
+    --    "1.42",
+    --    [[
+    --        SELECT a FROM t2 ORDER BY b DESC,c DESC,d,e,f LIMIT 31;
+    --    ]],
+    --    test:execsql "SELECT a FROM t2 ORDER BY +b DESC,+c DESC,+d,+e,+f LIMIT 31"
+    --    )
+    --
+    --test:do_execsql_test(
+    --    "1.43",
+    --    [[
+    --        SELECT a FROM t2 ORDER BY b,c,d,e,f DESC LIMIT 8 OFFSET 7;
+    --    ]],
+    --    test:execsql "SELECT a FROM t2 ORDER BY +b,+c,+d,+e,+f DESC LIMIT 8 OFFSET 7"
+    --    )
+    -- <end>
 test:finish_test()
diff --git a/test/sql-tap/orderby8.test.lua b/test/sql-tap/orderby8.test.lua
index 92fec6dbe..95acee1e1 100755
--- a/test/sql-tap/orderby8.test.lua
+++ b/test/sql-tap/orderby8.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(201)
+test:plan(203)
 
 --!./tcltestrunner.lua
 -- 2015-01-19
@@ -50,4 +50,29 @@ for i=1,200 do
     })
 end
 
+-- Tests to check that ORDER BY + LIMIT + (ASC + DESC, different
+-- sorting orders) are forbidden. Such combination is forbidden
+-- because currently it returns data in wrong sorting order.
+-- It will be fixed when multi-directional would be introduced:
+-- https://github.com/tarantool/tarantool/issues/3309
+
+test:do_catchsql_test(
+    "1.201",
+    [[
+        CREATE TABLE t2 (id INT PRIMARY KEY, a INT, b INT);
+        INSERT INTO t2 VALUES (1, 2, 1), (2, -3, 5), (3, 2, -3), (4, 2, 12);
+        SELECT * FROM t2 ORDER BY a ASC, b DESC LIMIT 5;
+    ]],
+    {1, "ORDER BY does not support different sorting orders"}
+)
+
+test:do_catchsql_test(
+    "1.202",
+    [[
+        SELECT * FROM t2 ORDER BY a, b DESC LIMIT 5;
+    ]],
+    {1, "ORDER BY does not support different sorting orders"}
+
+)
+
 test:finish_test()
> 
> A bit of code-style refactoring:
> 
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index 85719e3a1..bd494ef2e 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -1810,12 +1810,15 @@ sqlExprListSetSortOrder(struct ExprList *p, enum sort_order sort_order)
> }
> 
> void
> -sql_check_sort_orders(ExprList * expr_list, Parse *parse) {
> +sql_expr_check_sort_orders(struct Parse *parse,
> +                          const struct ExprList *expr_list)
> +{
>        if(expr_list == NULL)
>                return;
> -       for (int i = 0; i < expr_list->nExpr; i++) {
> +       enum sort_order reference_order = expr_list->a[0].sort_order;
> +       for (int i = 1; i < expr_list->nExpr; i++) {
>                assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
> -               if (expr_list->a[i].sort_order != expr_list->a[0].sort_order) {
> +               if (expr_list->a[i].sort_order != reference_order) {
>                        diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
>                                 "different sorting orders");
>                        sql_parser_error(parse);
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index 95d6962f9..f056a89b4 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -662,7 +662,7 @@ using_opt(U) ::= .                        {U = 0;}
> 
> orderby_opt(A) ::= .                          {A = 0;}
> orderby_opt(A) ::= ORDER BY sortlist(X). {
> -  sql_check_sort_orders(X, pParse);
> +  sql_check_sort_orders(pParse, X);
>   A = X;
> }
> sortlist(A) ::= sortlist(A) COMMA expr(Y) sortorder(Z). {
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 06818d9e1..324ab1618 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -3238,18 +3238,19 @@ ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *);
> void sqlExprListSetSortOrder(ExprList *, enum sort_order sort_order);
> 
> /**
> - * Check if sorting orders are the same in ORDER BY and rise an
> + * Check if sorting orders are the same in ORDER BY and raise an
>  * error if they are not.
>  *
>  * In future, we will support different sorting orders in
>  * ORDER BY (e.g. ORDER BY col1 ASC, col2 DESC) and remove this
> - * check (see ticket #3309).
> - * @param expr_list Expression list with  ORDER BY clause
> + * check.
> + *  * @param parse Parsing context.
> + * @param expr_list Expression list with ORDER BY clause
>  * at the end.
> - * @param parse Parsing context.
>  */
> void
> -sql_check_sort_orders(ExprList * expr_list, Parse *parse);
> +sql_expr_check_sort_orders(struct Parse *parse,
> +                          const struct ExprList *expr_list);
Thank you! Sorry that you had to do these fixes. Applied all of them.

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a75f23756..bd494ef2e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1809,6 +1809,24 @@ sqlExprListSetSortOrder(struct ExprList *p, enum sort_order sort_order)
 	p->a[p->nExpr - 1].sort_order = sort_order;
 }
 
+void
+sql_expr_check_sort_orders(struct Parse *parse,
+			   const struct ExprList *expr_list)
+{
+	if(expr_list == NULL)
+		return;
+	enum sort_order reference_order = expr_list->a[0].sort_order;
+	for (int i = 1; i < expr_list->nExpr; i++) {
+		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
+		if (expr_list->a[i].sort_order != reference_order) {
+			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
+				 "different sorting orders");
+			sql_parser_error(parse);
+			return;
+		}
+	}
+}
+

+/**
+ * Check if sorting orders are the same in ORDER BY and raise an
+ * error if they are not.
+ *
+ * This check is needed only for ORDER BY + LIMIT, because
+ * currently ORDER BY + LIMIT + ASC + DESC produces incorrectly
+ * sorted results and thus forbidden. In future, we will
+ * support different sorting orders in
+ * ORDER BY + LIMIT (e.g. ORDER BY col1 ASC, col2 DESC LIMIT ...)
+ * and remove this check.
+ * @param parse Parsing context.
+ * @param expr_list Expression list with  ORDER BY clause
+ * at the end.
+ */
+void
+sql_expr_check_sort_orders(struct Parse *parse,
+			   const struct ExprList *expr_list);
+

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-15 12:10   ` i.koptelov
@ 2019-03-15 16:23     ` n.pettik
  2019-03-15 16:52       ` i.koptelov
  0 siblings, 1 reply; 7+ messages in thread
From: n.pettik @ 2019-03-15 16:23 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Ivan Koptelov

[-- Attachment #1: Type: text/plain, Size: 2519 bytes --]


>> Plase, make up a list of tests to be enabled after multi-directional
>> iterators are introduced. Don’t fix tests which fail now, simply comment
>> them. Finally, add a few simple tests verifying that different sorting
>> orders are not supported now.
> 
> I did as you say. Added list of tests as a comment to #3309.
> But I would like to know, why commenting tests & making a list
> of them is better than change tests & comment old expected results?
> The main advantage is that one can not ignore/miss tests, which
> would fail after #3309 is done.

Firstly, they won’t automatically start to fail, since check is SQL
code won’t disappear itself. Secondly, such approach allows to
significantly reduce efforts to turn tests on/off after/before fixes.

> +    -- Tests are commented because of the reason described in
> +    -- NOTE at the beginning of the file.
> +    -- <begin>
> +    --local data = {
> +    --    {limit=0, offset=4, orderby="+b,+a"},
> +    --    {limit=0, offset=5, orderby="+b,+a"},
> +    --    {limit=0, offset=6, orderby="+b,+a"},
> +    --    {limit=0, offset=9, orderby="+b,+a"},
> +    --    {limit=0, offset=0, orderby="+b,+a"},
> +    --    {limit=0, offset=1, orderby="+b,+a"},
> +    --    {limit=7, offset=4, orderby="+b,+a"},
> +    --    {limit=7, offset=9, orderby="+b,+a”},

These first tests come with the same sorting orders,
can we avoid commenting them?

> +    --    {limit=0, offset=4, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=5, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=6, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=9, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=0, orderby="+b DESC,+a"},
> +    --    {limit=0, offset=1, orderby="+b DESC,+a"},
> +    --    {limit=7, offset=4, orderby="+b DESC,+a"},
> +    --    {limit=7, offset=9, orderby="+b DESC,+a"},
> 
> +void
> +sql_expr_check_sort_orders(struct Parse *parse,
> +			   const struct ExprList *expr_list)
> +{
> +	if(expr_list == NULL)
> +		return;
> +	enum sort_order reference_order = expr_list->a[0].sort_order;
> +	for (int i = 1; i < expr_list->nExpr; i++) {
> +		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
> +		if (expr_list->a[i].sort_order != reference_order) {
> +			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
> +				 "different sorting orders”);

Different sorting orders and LIMIT clause at the same time OR
“ORDER BY with LIMIT”, “different sorting orders”.


[-- Attachment #2: Type: text/html, Size: 40933 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-15 16:23     ` n.pettik
@ 2019-03-15 16:52       ` i.koptelov
  2019-03-15 16:55         ` n.pettik
  0 siblings, 1 reply; 7+ messages in thread
From: i.koptelov @ 2019-03-15 16:52 UTC (permalink / raw)
  To: tarantool-patches; +Cc: n.pettik



> On 15 Mar 2019, at 19:23, n.pettik <korablev@tarantool.org> wrote:
> 
>> 
>> +    -- Tests are commented because of the reason described in
>> +    -- NOTE at the beginning of the file.
>> +    -- <begin>
>> +    --local data = {
>> +    --    {limit=0, offset=4, orderby="+b,+a"},
>> +    --    {limit=0, offset=5, orderby="+b,+a"},
>> +    --    {limit=0, offset=6, orderby="+b,+a"},
>> +    --    {limit=0, offset=9, orderby="+b,+a"},
>> +    --    {limit=0, offset=0, orderby="+b,+a"},
>> +    --    {limit=0, offset=1, orderby="+b,+a"},
>> +    --    {limit=7, offset=4, orderby="+b,+a"},
>> +    --    {limit=7, offset=9, orderby="+b,+a”},
> 
> These first tests come with the same sorting orders,
> can we avoid commenting them?
Yes.

diff --git a/test/sql-tap/orderby6.test.lua b/test/sql-tap/orderby6.test.lua
index 7fe0032dd..e51059ffc 100755
--- a/test/sql-tap/orderby6.test.lua
+++ b/test/sql-tap/orderby6.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(18)
+test:plan(34)
 
 --!./tcltestrunner.lua
 -- 2014-03-21
@@ -158,48 +158,49 @@ testprefix = "orderby6"
     -- Many test cases where the LIMIT+OFFSET window is in various
     -- alignments with block-sort boundaries.
     --
-    -- Tests are commented because of the reason described in
-    -- NOTE at the beginning of the file.
-    -- <begin>
-    --local data = {
-    --    {limit=0, offset=4, orderby="+b,+a"},
-    --    {limit=0, offset=5, orderby="+b,+a"},
-    --    {limit=0, offset=6, orderby="+b,+a"},
-    --    {limit=0, offset=9, orderby="+b,+a"},
-    --    {limit=0, offset=0, orderby="+b,+a"},
-    --    {limit=0, offset=1, orderby="+b,+a"},
-    --    {limit=7, offset=4, orderby="+b,+a"},
-    --    {limit=7, offset=9, orderby="+b,+a"},
-    --    {limit=0, offset=4, orderby="+b DESC,+a"},
-    --    {limit=0, offset=5, orderby="+b DESC,+a"},
-    --    {limit=0, offset=6, orderby="+b DESC,+a"},
-    --    {limit=0, offset=9, orderby="+b DESC,+a"},
-    --    {limit=0, offset=0, orderby="+b DESC,+a"},
-    --    {limit=0, offset=1, orderby="+b DESC,+a"},
-    --    {limit=7, offset=4, orderby="+b DESC,+a"},
-    --    {limit=7, offset=9, orderby="+b DESC,+a"},
-    --    {limit=0, offset=4, orderby="+b,+a DESC"},
-    --    {limit=0, offset=5, orderby="+b,+a DESC"},
-    --    {limit=0, offset=6, orderby="+b,+a DESC"},
-    --    {limit=0, offset=9, orderby="+b,+a DESC"},
-    --    {limit=0, offset=0, orderby="+b,+a DESC"},
-    --    {limit=0, offset=1, orderby="+b,+a DESC"},
-    --    {limit=7, offset=4, orderby="+b,+a DESC"},
-    --    {limit=7, offset=9, orderby="+b,+a DESC"},
-    --    {limit=0, offset=4, orderby="+b DESC,+a DESC"},
-    --    {limit=0, offset=5, orderby="+b DESC,+a DESC"},
-    --    {limit=0, offset=6, orderby="+b DESC,+a DESC"},
-    --    {limit=0, offset=9, orderby="+b DESC,+a DESC"},
-    --    {limit=0, offset=0, orderby="+b DESC,+a DESC"},
-    --    {limit=0, offset=1, orderby="+b DESC,+a DESC"},
-    --    {limit=7, offset=4, orderby="+b DESC,+a DESC"},
-    --    {limit=7, offset=9, orderby="+b DESC,+a DESC"}}
-    --for i, v in ipairs(data) do
-    --    local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-    --    local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
-    --    test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
-    --end
-    -- <end>
+    local data = {
+        {limit=0, offset=4, orderby="+b,+a"},
+        {limit=0, offset=5, orderby="+b,+a"},
+        {limit=0, offset=6, orderby="+b,+a"},
+        {limit=0, offset=9, orderby="+b,+a"},
+        {limit=0, offset=0, orderby="+b,+a"},
+        {limit=0, offset=1, orderby="+b,+a"},
+        {limit=7, offset=4, orderby="+b,+a"},
+        {limit=7, offset=9, orderby="+b,+a"},
+        -- Tests with different sorting orders are commented
+        -- because of the reason described in NOTE at the
+        -- beginning of the file.
+        -- <begin>
+        --{limit=0, offset=4, orderby="+b DESC,+a"},
+        --{limit=0, offset=5, orderby="+b DESC,+a"},
+        --{limit=0, offset=6, orderby="+b DESC,+a"},
+        --{limit=0, offset=9, orderby="+b DESC,+a"},
+        --{limit=0, offset=0, orderby="+b DESC,+a"},
+        --{limit=0, offset=1, orderby="+b DESC,+a"},
+        --{limit=7, offset=4, orderby="+b DESC,+a"},
+        --{limit=7, offset=9, orderby="+b DESC,+a"},
+        --{limit=0, offset=4, orderby="+b,+a DESC"},
+        --{limit=0, offset=5, orderby="+b,+a DESC"},
+        --{limit=0, offset=6, orderby="+b,+a DESC"},
+        --{limit=0, offset=9, orderby="+b,+a DESC"},
+        --{limit=0, offset=0, orderby="+b,+a DESC"},
+        --{limit=0, offset=1, orderby="+b,+a DESC"},
+        --{limit=7, offset=4, orderby="+b,+a DESC"},
+        --{limit=7, offset=9, orderby="+b,+a DESC”},
+        -- <end>
+        {limit=0, offset=4, orderby="+b DESC,+a DESC"},
+        {limit=0, offset=5, orderby="+b DESC,+a DESC"},
+        {limit=0, offset=6, orderby="+b DESC,+a DESC"},
+        {limit=0, offset=9, orderby="+b DESC,+a DESC"},
+        {limit=0, offset=0, orderby="+b DESC,+a DESC"},
+        {limit=0, offset=1, orderby="+b DESC,+a DESC"},
+        {limit=7, offset=4, orderby="+b DESC,+a DESC"},
+        {limit=7, offset=9, orderby="+b DESC,+a DESC"}}
+    for i, v in ipairs(data) do
+        local sql1 = "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+        local sql2 = "SELECT a FROM t1 ORDER BY "..string.gsub(v.orderby, "+", "").." LIMIT "..v.limit.." OFFSET "..v.offset..";"
+        test:do_execsql_test("1.21."..i, sql1, test:execsql(sql2))
+    end
> 
>> +    --    {limit=0, offset=4, orderby="+b DESC,+a"},
>> +    --    {limit=0, offset=5, orderby="+b DESC,+a"},
>> +    --    {limit=0, offset=6, orderby="+b DESC,+a"},
>> +    --    {limit=0, offset=9, orderby="+b DESC,+a"},
>> +    --    {limit=0, offset=0, orderby="+b DESC,+a"},
>> +    --    {limit=0, offset=1, orderby="+b DESC,+a"},
>> +    --    {limit=7, offset=4, orderby="+b DESC,+a"},
>> +    --    {limit=7, offset=9, orderby="+b DESC,+a"},
>> 
>> +void
>> +sql_expr_check_sort_orders(struct Parse *parse,
>> +			   const struct ExprList *expr_list)
>> +{
>> +	if(expr_list == NULL)
>> +		return;
>> +	enum sort_order reference_order = expr_list->a[0].sort_order;
>> +	for (int i = 1; i < expr_list->nExpr; i++) {
>> +		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
>> +		if (expr_list->a[i].sort_order != reference_order) {
>> +			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
>> +				 "different sorting orders”);
> 
> Different sorting orders and LIMIT clause at the same time OR
> “ORDER BY with LIMIT”, “different sorting orders”.

Thank you for noting. Fixed:

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index bd494ef2e..511e74c65 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -1819,7 +1819,8 @@ sql_expr_check_sort_orders(struct Parse *parse,
 	for (int i = 1; i < expr_list->nExpr; i++) {
 		assert(expr_list->a[i].sort_order != SORT_ORDER_UNDEF);
 		if (expr_list->a[i].sort_order != reference_order) {
-			diag_set(ClientError, ER_UNSUPPORTED, "ORDER BY",
+			diag_set(ClientError, ER_UNSUPPORTED,
+				 "ORDER BY with LIMIT",
 				 "different sorting orders");
 			sql_parser_error(parse);
 			return;

diff --git a/test/sql-tap/orderby8.test.lua b/test/sql-tap/orderby8.test.lua
index 95acee1e1..52f125755 100755
--- a/test/sql-tap/orderby8.test.lua
+++ b/test/sql-tap/orderby8.test.lua
@@ -63,7 +63,7 @@ test:do_catchsql_test(
         INSERT INTO t2 VALUES (1, 2, 1), (2, -3, 5), (3, 2, -3), (4, 2, 12);
         SELECT * FROM t2 ORDER BY a ASC, b DESC LIMIT 5;
     ]],
-    {1, "ORDER BY does not support different sorting orders"}
+    {1, "ORDER BY with LIMIT does not support different sorting orders"}
 )
 
 test:do_catchsql_test(
@@ -71,7 +71,7 @@ test:do_catchsql_test(
     [[
         SELECT * FROM t2 ORDER BY a, b DESC LIMIT 5;
     ]],
-    {1, "ORDER BY does not support different sorting orders"}
+    {1, "ORDER BY with LIMIT does not support different sorting orders"}
 
 )

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-15 16:52       ` i.koptelov
@ 2019-03-15 16:55         ` n.pettik
  0 siblings, 0 replies; 7+ messages in thread
From: n.pettik @ 2019-03-15 16:55 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Ivan Koptelov


LGTM.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY
  2019-03-11 15:44 [tarantool-patches] [PATCH] sql: forbid different sorting orders in ORDER BY Ivan Koptelov
  2019-03-12 14:25 ` [tarantool-patches] " n.pettik
@ 2019-03-19 11:25 ` Kirill Yukhin
  1 sibling, 0 replies; 7+ messages in thread
From: Kirill Yukhin @ 2019-03-19 11:25 UTC (permalink / raw)
  To: tarantool-patches; +Cc: korablev, Ivan Koptelov

Hello,

On 11 Mar 18:44, Ivan Koptelov wrote:
> Without multi-directional iterators (to be introduced in #3309)
> in most cases (except those when internal VDBE merge sorter is
> used) ORDER BY with different specified ordered leads to wrong
> result. So for now (till #3309 is resolved) different sort
> orders are forbidden.
> 
> Close #4038
> 
> @TarantoolBot document
> Title: different sorting orders in ORDER BY are forbidden now
> The error will be raised if different sorting orders are
> encountered.
> ---
> Branch https://github.com/tarantool/tarantool/tree/sudobobo/gh-4038-ban-diff-orders-in-ORDER-BY
> Issue https://github.com/tarantool/tarantool/issues/4038

I've checked your patch into 2.1 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-19 11:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 15:44 [tarantool-patches] [PATCH] sql: forbid different sorting orders in ORDER BY Ivan Koptelov
2019-03-12 14:25 ` [tarantool-patches] " n.pettik
2019-03-15 12:10   ` i.koptelov
2019-03-15 16:23     ` n.pettik
2019-03-15 16:52       ` i.koptelov
2019-03-15 16:55         ` n.pettik
2019-03-19 11:25 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox