From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id EF57E27EB0 for ; Fri, 15 Mar 2019 12:52:12 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KJPSi2pSjCgd for ; Fri, 15 Mar 2019 12:52:12 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 0EAA227E66 for ; Fri, 15 Mar 2019 12:52:11 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY From: "i.koptelov" In-Reply-To: Date: Fri, 15 Mar 2019 19:52:09 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <3E23DEAF-464D-4901-BF9D-CDC40CC08682@tarantool.org> References: <20190311154448.73563-1-ivan.koptelov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: "n.pettik" > On 15 Mar 2019, at 19:23, n.pettik wrote: >=20 >>=20 >> + -- Tests are commented because of the reason described in >> + -- NOTE at the beginning of the file. >> + -- >> + --local data =3D { >> + -- {limit=3D0, offset=3D4, orderby=3D"+b,+a"}, >> + -- {limit=3D0, offset=3D5, orderby=3D"+b,+a"}, >> + -- {limit=3D0, offset=3D6, orderby=3D"+b,+a"}, >> + -- {limit=3D0, offset=3D9, orderby=3D"+b,+a"}, >> + -- {limit=3D0, offset=3D0, orderby=3D"+b,+a"}, >> + -- {limit=3D0, offset=3D1, orderby=3D"+b,+a"}, >> + -- {limit=3D7, offset=3D4, orderby=3D"+b,+a"}, >> + -- {limit=3D7, offset=3D9, orderby=3D"+b,+a=E2=80=9D}, >=20 > 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 =3D require("sqltester") -test:plan(18) +test:plan(34) =20 --!./tcltestrunner.lua -- 2014-03-21 @@ -158,48 +158,49 @@ testprefix =3D "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. - -- - --local data =3D { - -- {limit=3D0, offset=3D4, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D5, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D6, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D9, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D0, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D1, orderby=3D"+b,+a"}, - -- {limit=3D7, offset=3D4, orderby=3D"+b,+a"}, - -- {limit=3D7, offset=3D9, orderby=3D"+b,+a"}, - -- {limit=3D0, offset=3D4, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D5, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D6, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D9, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D0, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D1, orderby=3D"+b DESC,+a"}, - -- {limit=3D7, offset=3D4, orderby=3D"+b DESC,+a"}, - -- {limit=3D7, offset=3D9, orderby=3D"+b DESC,+a"}, - -- {limit=3D0, offset=3D4, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D5, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D6, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D9, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D0, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D1, orderby=3D"+b,+a DESC"}, - -- {limit=3D7, offset=3D4, orderby=3D"+b,+a DESC"}, - -- {limit=3D7, offset=3D9, orderby=3D"+b,+a DESC"}, - -- {limit=3D0, offset=3D4, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D0, offset=3D5, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D0, offset=3D6, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D0, offset=3D9, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D0, offset=3D0, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D0, offset=3D1, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D7, offset=3D4, orderby=3D"+b DESC,+a DESC"}, - -- {limit=3D7, offset=3D9, orderby=3D"+b DESC,+a DESC"}} - --for i, v in ipairs(data) do - -- local sql1 =3D "SELECT a FROM t1 ORDER BY "..v.orderby.." = LIMIT "..v.limit.." OFFSET "..v.offset..";" - -- local sql2 =3D "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 - -- + local data =3D { + {limit=3D0, offset=3D4, orderby=3D"+b,+a"}, + {limit=3D0, offset=3D5, orderby=3D"+b,+a"}, + {limit=3D0, offset=3D6, orderby=3D"+b,+a"}, + {limit=3D0, offset=3D9, orderby=3D"+b,+a"}, + {limit=3D0, offset=3D0, orderby=3D"+b,+a"}, + {limit=3D0, offset=3D1, orderby=3D"+b,+a"}, + {limit=3D7, offset=3D4, orderby=3D"+b,+a"}, + {limit=3D7, offset=3D9, orderby=3D"+b,+a"}, + -- Tests with different sorting orders are commented + -- because of the reason described in NOTE at the + -- beginning of the file. + -- + --{limit=3D0, offset=3D4, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D5, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D6, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D9, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D0, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D1, orderby=3D"+b DESC,+a"}, + --{limit=3D7, offset=3D4, orderby=3D"+b DESC,+a"}, + --{limit=3D7, offset=3D9, orderby=3D"+b DESC,+a"}, + --{limit=3D0, offset=3D4, orderby=3D"+b,+a DESC"}, + --{limit=3D0, offset=3D5, orderby=3D"+b,+a DESC"}, + --{limit=3D0, offset=3D6, orderby=3D"+b,+a DESC"}, + --{limit=3D0, offset=3D9, orderby=3D"+b,+a DESC"}, + --{limit=3D0, offset=3D0, orderby=3D"+b,+a DESC"}, + --{limit=3D0, offset=3D1, orderby=3D"+b,+a DESC"}, + --{limit=3D7, offset=3D4, orderby=3D"+b,+a DESC"}, + --{limit=3D7, offset=3D9, orderby=3D"+b,+a DESC=E2=80=9D}, + -- + {limit=3D0, offset=3D4, orderby=3D"+b DESC,+a DESC"}, + {limit=3D0, offset=3D5, orderby=3D"+b DESC,+a DESC"}, + {limit=3D0, offset=3D6, orderby=3D"+b DESC,+a DESC"}, + {limit=3D0, offset=3D9, orderby=3D"+b DESC,+a DESC"}, + {limit=3D0, offset=3D0, orderby=3D"+b DESC,+a DESC"}, + {limit=3D0, offset=3D1, orderby=3D"+b DESC,+a DESC"}, + {limit=3D7, offset=3D4, orderby=3D"+b DESC,+a DESC"}, + {limit=3D7, offset=3D9, orderby=3D"+b DESC,+a DESC"}} + for i, v in ipairs(data) do + local sql1 =3D "SELECT a FROM t1 ORDER BY "..v.orderby.." LIMIT = "..v.limit.." OFFSET "..v.offset..";" + local sql2 =3D "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 >=20 >> + -- {limit=3D0, offset=3D4, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D0, offset=3D5, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D0, offset=3D6, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D0, offset=3D9, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D0, offset=3D0, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D0, offset=3D1, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D7, offset=3D4, orderby=3D"+b DESC,+a"}, >> + -- {limit=3D7, offset=3D9, orderby=3D"+b DESC,+a"}, >>=20 >> +void >> +sql_expr_check_sort_orders(struct Parse *parse, >> + const struct ExprList *expr_list) >> +{ >> + if(expr_list =3D=3D NULL) >> + return; >> + enum sort_order reference_order =3D expr_list->a[0].sort_order; >> + for (int i =3D 1; i < expr_list->nExpr; i++) { >> + assert(expr_list->a[i].sort_order !=3D = SORT_ORDER_UNDEF); >> + if (expr_list->a[i].sort_order !=3D reference_order) { >> + diag_set(ClientError, ER_UNSUPPORTED, "ORDER = BY", >> + "different sorting orders=E2=80=9D); >=20 > Different sorting orders and LIMIT clause at the same time OR > =E2=80=9CORDER BY with LIMIT=E2=80=9D, =E2=80=9Cdifferent sorting = orders=E2=80=9D. 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 =3D 1; i < expr_list->nExpr; i++) { assert(expr_list->a[i].sort_order !=3D = SORT_ORDER_UNDEF); if (expr_list->a[i].sort_order !=3D 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"} ) =20 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"} =20 )=