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 74BCF2A81A for ; Tue, 11 Sep 2018 17:33:51 -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 T4qC9uLp9BlT for ; Tue, 11 Sep 2018 17:33:51 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 B207F2A813 for ; Tue, 11 Sep 2018 17:33:50 -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" In-Reply-To: Date: Wed, 12 Sep 2018 00:33:44 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: 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: Kirill Shcherbatov > Decreased the maximum number of terms in a compound SELECT > statement. The code generator for compound SELECT statements > does one level of recursion for each term. A stack overflow can > result if the number of terms is too large. In practice, most > SQL never has more than 3 or 4 terms. It is quite dubious statement without proofs. I guess you borrowed it from comments in source code, but anyway lets drop it from commit = message. * It may be applied to SQLite which is considered as light db to be used on mobile devices etc * > Fiber stack is 64KB by default, so maximum number of entities > should be about 30 to stack guard will not be triggered. Why did you choose 30? AFAIU previous =E2=80=9950=E2=80=99 also was = taken as 'stack guard will not be triggered=E2=80=99 number. Is this just typical =E2=80=98less than 50 but not too much=E2=80=99?:) Have you checked this number on different compilers/OSs? >=20 > Closes #3382. > --- > Branch: = http://github.com/tarantool/tarantool/tree/kshsh/gh-3382-select-compound-l= imit-fix > Issue: https://github.com/tarantool/tarantool/issues/3382 >=20 > src/box/sql/parse.y | 4 +++- > src/box/sql/sqliteLimit.h | 8 ++++---- > test/sql-tap/gh2548-select-compound-limit.test.lua | 17 = +++++++++++++++-- > test/sql-tap/select7.test.lua | 2 +- > 4 files changed, 23 insertions(+), 8 deletions(-) >=20 > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index d8532d3..c5e90a7 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -423,7 +423,9 @@ cmd ::=3D select(X). { > (mxSelect =3D = pParse->db->aLimit[SQLITE_LIMIT_COMPOUND_SELECT])>0 && > cnt>mxSelect > ){ > - sqlite3ErrorMsg(pParse, "Too many UNION or EXCEPT or = INTERSECT operations"); > + sqlite3ErrorMsg(pParse, "Too many UNION or EXCEPT or = INTERSECT " > + "operations (limit %d is set)", > + = pParse->db->aLimit[SQLITE_LIMIT_COMPOUND_SELECT]); Mb it is worth to add pragma in order to adjust number of max nested = selects? I ask this because error message gives an illusion of such opportunity ("if limit is set, then I can change it=E2=80=9D). Or change message to = simple: (limit is %d). It is only request, I like this additional information = btw. Also, another cool option would be or . It would display all current settings and options: max number of nested = selects, depth of recursion in triggers etc. > } > } > } > diff --git a/src/box/sql/sqliteLimit.h b/src/box/sql/sqliteLimit.h > index b88c9c6..0b8fc43 100644 > --- a/src/box/sql/sqliteLimit.h > +++ b/src/box/sql/sqliteLimit.h > @@ -117,12 +117,12 @@ enum { > * never has more than 3 or 4 terms. Use a value of 0 to disable > * any limit on the number of terms in a compount SELECT. Is this comment relevant? I know that it is trapped here accidentally, but lets remove it in case it is outdated.=20 I found this (https://www.sqlite.org/lang_select.html): =E2=80=98'' The VALUES clause The phrase "VALUES(expr-list)" means the same thing as "SELECT = expr-list=E2=80=9D. The phrase "VALUES(expr-list-1),...,(expr-list-N)" means the same thing = as "SELECT expr-list-1 UNION ALL ... UNION ALL SELECT expr-list-N=E2=80=9D. Both forms are the same, except that the number of SELECT statements in a compound is limited bySQLITE_LIMIT_COMPOUND_SELECT whereas the number of rows in a VALUES clause has no arbitrary limit. =E2=80=98=E2=80=99' Do we have tests on this case? > * > - * Tarantool: gh-2548: Fiber stack is 64KB by default, so maximum > - * number of entities should be less than 50 or stack guard will be > - * triggered. > + * Tarantool: gh-2548, gh-3382: Fiber stack is 64KB by default, > + * so maximum number of entities should be less than 30 or stack > + * guard will be triggered. It is so ridiculous :)) Never say never.