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 5C490270B4 for ; Fri, 15 Mar 2019 12:23:25 -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 pX7z1I9jAzvF for ; Fri, 15 Mar 2019 12:23:25 -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 9875726EA1 for ; Fri, 15 Mar 2019 12:23:24 -0400 (EDT) From: "n.pettik" Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_D318C3ED-C2C8-42B3-AA05-02D5301B7DB8" Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] sql: forbid different sorting orders in ORDER BY Date: Fri, 15 Mar 2019 19:23:22 +0300 In-Reply-To: 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: Ivan Koptelov --Apple-Mail=_D318C3ED-C2C8-42B3-AA05-02D5301B7DB8 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 >> Plase, make up a list of tests to be enabled after multi-directional >> iterators are introduced. Don=E2=80=99t fix tests which fail now, = simply comment >> them. Finally, add a few simple tests verifying that different = sorting >> orders are not supported now. >=20 > 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=E2=80=99t automatically start to fail, since check is = SQL code won=E2=80=99t 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. > + -- > + --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}, These first tests come with the same sorting orders, can we avoid commenting them? > + -- {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); 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. --Apple-Mail=_D318C3ED-C2C8-42B3-AA05-02D5301B7DB8 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

Plase, = make up a list of tests to be enabled after multi-directional
iterators are introduced. Don=E2=80=99t 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=E2=80=99t automatically start to = fail, since check is SQL
code won=E2=80=99t 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 =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},

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

+    --    {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"},

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

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.

= --Apple-Mail=_D318C3ED-C2C8-42B3-AA05-02D5301B7DB8--