Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org
Cc: korablev@tarantool.org, Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [tarantool-patches] [PATCH v1 1/1] sql: decrease SELECT_COMPOUND_LIMIT threshold
Date: Tue, 11 Sep 2018 14:47:14 +0300	[thread overview]
Message-ID: <d67026a5945783bd669e393ea0a839ce9b35983c.1536666375.git.kshcherbatov@tarantool.org> (raw)

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.
Fiber stack is 64KB by default, so maximum number of entities
should be about 30 to stack guard will not be triggered.

Closes #3382.
---
Branch: http://github.com/tarantool/tarantool/tree/kshsh/gh-3382-select-compound-limit-fix
Issue: https://github.com/tarantool/tarantool/issues/3382

 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(-)

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 ::= select(X).  {
         (mxSelect = 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]);
       }
     }
   }
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.
  *
- * 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.
  */
 #ifndef SQLITE_MAX_COMPOUND_SELECT
-#define SQLITE_MAX_COMPOUND_SELECT 50
+#define SQLITE_MAX_COMPOUND_SELECT 30
 #endif
 
 /*
diff --git a/test/sql-tap/gh2548-select-compound-limit.test.lua b/test/sql-tap/gh2548-select-compound-limit.test.lua
index 9cfc066..6546f1d 100755
--- a/test/sql-tap/gh2548-select-compound-limit.test.lua
+++ b/test/sql-tap/gh2548-select-compound-limit.test.lua
@@ -1,10 +1,12 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(8)
+test:plan(9)
 
 -- box.cfg{wal_mode='none'}
 
-table_count = 51
+table_count = 31
+
+select_string_last = ''
 
 for _, term in ipairs({'UNION', 'UNION ALL', 'INTERSECT', 'EXCEPT'}) do 
     select_string = ''
@@ -39,6 +41,8 @@ for _, term in ipairs({'UNION', 'UNION ALL', 'INTERSECT', 'EXCEPT'}) do
                  end,
                  false)
 
+    select_string_last = select_string
+
 --    if not pcall(function() box.sql.execute(select_string) end) then
 --        print('not ok')
 --    end
@@ -49,4 +53,13 @@ for _, term in ipairs({'UNION', 'UNION ALL', 'INTERSECT', 'EXCEPT'}) do
 --    end
 end
 
+
+test:do_catchsql_test(
+    "gh2548-select-compound-limit-2",
+    select_string_last, {
+        -- <gh2548-select-compound-limit-2>
+        1, "Too many UNION or EXCEPT or INTERSECT operations (limit 30 is set)"
+        -- </gh2548-select-compound-limit-2>
+    })
+
 test:finish_test()
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index 10e13e2..59465f0 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -179,7 +179,7 @@ test:do_catchsql_test(
     "select7-6.2",
     sql, {
         -- <select7-6.2>
-        1, "Too many UNION or EXCEPT or INTERSECT operations"
+        1, "Too many UNION or EXCEPT or INTERSECT operations (limit 30 is set)"
         -- </select7-6.2>
     })
 
-- 
2.7.4

             reply	other threads:[~2018-09-11 11:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-11 11:47 Kirill Shcherbatov [this message]
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
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=d67026a5945783bd669e393ea0a839ce9b35983c.1536666375.git.kshcherbatov@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [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