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 EB55629B49
	for <tarantool-patches@freelists.org>; Thu, 20 Sep 2018 04:49:09 -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 2U5gmGxA2a0g for <tarantool-patches@freelists.org>;
	Thu, 20 Sep 2018 04:49:09 -0400 (EDT)
Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41])
	(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 54C4824C4C
	for <tarantool-patches@freelists.org>; Thu, 20 Sep 2018 04:49:09 -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 v1 1/1] sql: decrease
 SELECT_COMPOUND_LIMIT threshold
From: "n.pettik" <korablev@tarantool.org>
In-Reply-To: <eaf45667-f956-d4ed-c368-3f11dec82be2@tarantool.org>
Date: Thu, 20 Sep 2018 11:49:01 +0300
Content-Transfer-Encoding: quoted-printable
Message-Id: <3E57AE93-5749-44DB-9146-034CBCD18896@tarantool.org>
References: <d67026a5945783bd669e393ea0a839ce9b35983c.1536666375.git.kshcherbatov@tarantool.org>
 <DD5AA202-0B4B-4DDB-A4F6-03D911483D9B@tarantool.org>
 <53ae8b41-f859-4296-71e5-c424e5f285fa@tarantool.org>
 <79E2AE74-BDE1-4E03-9345-F7DD863F811B@tarantool.org>
 <eaf45667-f956-d4ed-c368-3f11dec82be2@tarantool.org>
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: Kirill Shcherbatov <kshcherbatov@tarantool.org>


>> Hard limit 500 seems to be too much. Lets set hard limit 50 (as it =
was before).
>> If user want to set more than 50, he can set it to 0 (according to =
comments
>> it means no limit at all).
> Ok, set 50.
>=20
>> Ok, you may skip it, but anyway, I would add tests where you set
>> limit to 0 and process compound select with 31 parts.
> Introduced such test.
>=20
>> You forgot to fix commit message. See previous review comments.
> Updated.
>=20
>> This pragma status is not displayed via simple =E2=80=98pragma=E2=80=99=
.
> It is ok, the other non-boolean pragmas have same behavior.=20
> Displaying sql_default_engine was a big mistake. I've just implemented =
requirement
> from review comment that time, but it was not correct. It looks =
out-of-order,
> as an exception in code there.

I don=E2=80=99t think so. It looks OK and it (default engine) seems to
be very useful for user. However, I don=E2=80=99t insist on this change,
so let it be as is.

> @@ -193,7 +193,7 @@ test:do_execsql_test(
>         INSERT INTO t3 VALUES(56.0);
>     ]], {
>         -- <select7-7.1>
> -       =20
> +
>         -- </select7-7.1>
>     })
>=20
> @@ -216,7 +216,7 @@ test:do_execsql_test(
>         INSERT INTO t4 VALUES( 3.0 );
>     ]], {
>         -- <select7-7.3>
> -       =20
> +
>         -- </select7-7.3>
>     })
>=20
> @@ -233,7 +233,7 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "select7-7.5",
>     [[
> -        SELECT a=3D0, typeof(a) FROM t4=20
> +        SELECT a=3D0, typeof(a) FROM t4
>     ]], {
>         -- <select7-7.5>
>         0, "real", 0, "real"
> @@ -243,7 +243,7 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "select7-7.6",
>     [[
> -        SELECT a=3D0, typeof(a) FROM t4 GROUP BY a=20
> +        SELECT a=3D0, typeof(a) FROM t4 GROUP BY a
>     ]], {

Garbage diff.

Otherwise, patch LGTM.