From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: decrease SELECT_COMPOUND_LIMIT threshold
Date: Thu, 20 Sep 2018 11:49:01 +0300 [thread overview]
Message-ID: <3E57AE93-5749-44DB-9146-034CBCD18896@tarantool.org> (raw)
In-Reply-To: <eaf45667-f956-d4ed-c368-3f11dec82be2@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.
>
>> 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.
>
>> You forgot to fix commit message. See previous review comments.
> Updated.
>
>> This pragma status is not displayed via simple ‘pragma’.
> It is ok, the other non-boolean pragmas have same behavior.
> 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’t think so. It looks OK and it (default engine) seems to
be very useful for user. However, I don’t 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>
> -
> +
> -- </select7-7.1>
> })
>
> @@ -216,7 +216,7 @@ test:do_execsql_test(
> INSERT INTO t4 VALUES( 3.0 );
> ]], {
> -- <select7-7.3>
> -
> +
> -- </select7-7.3>
> })
>
> @@ -233,7 +233,7 @@ test:do_execsql_test(
> test:do_execsql_test(
> "select7-7.5",
> [[
> - SELECT a=0, typeof(a) FROM t4
> + SELECT a=0, 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=0, typeof(a) FROM t4 GROUP BY a
> + SELECT a=0, typeof(a) FROM t4 GROUP BY a
> ]], {
Garbage diff.
Otherwise, patch LGTM.
next prev parent reply other threads:[~2018-09-20 8:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-11 11:47 [tarantool-patches] " Kirill Shcherbatov
2018-09-11 21:33 ` [tarantool-patches] " n.pettik
2018-09-13 8:42 ` Kirill Shcherbatov
2018-09-19 21:29 ` n.pettik
2018-09-20 8:32 ` Kirill Shcherbatov
2018-09-20 8:49 ` n.pettik [this message]
2018-09-20 15:58 ` Kirill Yukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3E57AE93-5749-44DB-9146-034CBCD18896@tarantool.org \
--to=korablev@tarantool.org \
--cc=kshcherbatov@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: decrease SELECT_COMPOUND_LIMIT threshold' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox