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