From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 732C72787A
	for <tarantool-patches@freelists.org>; Wed, 18 Jul 2018 16:36:41 -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 BEiqymy4lXXh for <tarantool-patches@freelists.org>;
	Wed, 18 Jul 2018 16:36:41 -0400 (EDT)
Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94])
	(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 2E5E823554
	for <tarantool-patches@freelists.org>; Wed, 18 Jul 2018 16:36:41 -0400 (EDT)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: [tarantool-patches] Re: [PATCH] sql: assertion fail on nested select
From: "n.pettik" <korablev@tarantool.org>
In-Reply-To: <1530533500-9666-1-git-send-email-hollow653@gmail.com>
Date: Wed, 18 Jul 2018 23:36:38 +0300
Content-Transfer-Encoding: quoted-printable
Message-Id: <A93655E5-CB38-41D9-AA7A-872CD8D9F77E@tarantool.org>
References: <1530533500-9666-1-git-send-email-hollow653@gmail.com>
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-help: <mailto:ecartis@freelists.org?Subject=help>
List-unsubscribe: <tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-subscribe: <tarantool-patches-request@freelists.org?Subject=subscribe>
List-owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-archive: <http://www.freelists.org/archives/tarantool-patches>
To: tarantool-patches@freelists.org
Cc: "N. Tatunov" <hollow653@gmail.com>


> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index 54f78a9..c035691 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -3893,14 +3893,14 @@ flattenSubquery(Parse * pParse,		=
/* Parsing context */
> 	 * queries.
> 	 */
> 	if (pSub->pPrior) {
> -		if (pSub->pOrderBy) {
> -			return 0;	/* Restriction 20 */
> -		}
> 		if (isAgg || (p->selFlags & SF_Distinct) !=3D 0
> 		    || pSrc->nSrc !=3D 1) {
> 			return 0;
> 		}
> 		for (pSub1 =3D pSub; pSub1; pSub1 =3D pSub1->pPrior) {
> +			if (pSub1->pOrderBy) {
> +				return 0;	/* Restriction 20 */
> +			}

According to our code style:
- use explicit !=3D NULL comparison;
- don=E2=80=99t put bracers around one-line if statement;
- put comment above the code to be commented;

>diff --git a/test/sql-tap/subquery.test.lua =
b/test/sql-tap/subquery.test.lua
>--- a/test/sql-tap/subquery.test.lua
>+++ b/test/sql-tap/subquery.test.lua
>+test:do_execsql_test(
>+       "subquery-9.2",
>+       [[
>+               SELECT 'abc' FROM (SELECT * FROM table1 UNION ALL
>+                                   SELECT * FROM table1 UNION ALL
>+                                  SELECT * FROM table1 ORDER BY 1);
>+       ]], {
>+               -- <subquery-9.2>
>+               'abc','abc','abc','abc','abc','abc'
>+               -- <subquery-9.2>
>+       })

Why did you use here SELECT =E2=80=98abc=E2=80=99 and in other tests - =
SELECT *?
It is nitpicking, but just tests look a little weird..=