From: Nikita Pettik <korablev@tarantool.org> To: Kirill Shcherbatov <kshcherbatov@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: disallow ck using non-persistent function Date: Wed, 11 Sep 2019 00:22:37 +0300 [thread overview] Message-ID: <20190910212237.GA27128@tarantool.org> (raw) In-Reply-To: <33fa81dad5745292133063d39a3c20144718bb60.1567496399.git.kshcherbatov@tarantool.org> On Tue, Sep 03, 2019 at 10:40:43AM +0300, Kirill Shcherbatov wrote: > Each CK constraint object is a part of the database schema and > is restored during recovery. It is not possible if a CK > constraint uses some function inside. Nit: not some, but user-defined (built-ins are still available). >Thus we should disallow > non-persistent functions participate in ck constraints. > > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index 6f625dc18..3b4f01588 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -653,6 +653,18 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) > pExpr->iTable = func->def->name[0] == 'u' ? > 8388608 : 125829120; > } > + if ((pNC->ncFlags & NC_IsCheck) != 0 && > + func->def->body == NULL && > + func->def->language != FUNC_LANGUAGE_SQL_BUILTIN) { > + diag_set(ClientError, ER_SQL_PARSER_GENERIC, > + tt_sprintf("ck constraint could not " > + "use non-persistent " > + "function '%s'", > + func->def->name)); > + Nit: "Check constraint can not invoke non-persistent function" - sounds better. > pParse->is_aborted = true; > + pNC->nErr++; > + return WRC_Abort; > + } Ok, but still user can create persistent function, create check constraint involving that function and then drop function. Voila - Tarantool crashes. box.schema.func.create("MYFUNC", {exports = {'LUA', 'SQL'}, param_list = {'integer'}, body = "function(x) return x < 10 end"}) box.execute("CREATE TABLE t6(a INT CHECK (myfunc(a)) primary key);"); box.schema.func.drop("MYFUNC") box.space.T6:insert({1}) Segmentation fault code: 0 addr: 0x0 context: 0x11197e9c8 siginfo: 0x11197e960 Unfortunatelly, function is not bound to space(s), so we can't rebuild only affected check constraints (like during format change). The only option in this case rebuild all check constraints, but this solution doesn't seem to be acceptable. On the other hand, we can use the same approatch as for views: hold ar eference counter in each function and increment it each time when function is involved in check constraint (and in the future - in trigger). And disallow dropping function if its reference counter doesn't equal to zero. Please, come up with alternatives to this approach. > diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua > index cde213f8b..8ec3df84e 100644 > --- a/test/sql/checks.test.lua > +++ b/test/sql/checks.test.lua > @@ -235,3 +235,20 @@ physics_ck > physics_ck:drop() > > test_run:cmd("clear filter") > + > +-- > +-- gh-4176: Can't recover if check constraint involves function. > +-- Nit: please, write comment to test in form of "Make sure that ...". For instance: "Make sure that non-persistent functions can't participate in check constraints, since after instance reboot they disappear and check constraint can't be created." Using this wording allows to avoid > +function myfunc(x) return x < 10 end > +box.schema.func.create("MYFUNC", {exports = {'LUA', 'SQL'}, param_list = {'integer'}}) > +box.execute("CREATE TABLE t6(a INT CHECK (myfunc(a)) primary key);"); > +box.func.MYFUNC:drop() > + > +box.schema.func.create("MYFUNC", {exports = {'LUA', 'SQL'}, param_list = {'integer'}, body = "function(x) return x < 10 end"}) > +box.execute("CREATE TABLE t6(a INT CHECK (myfunc(a)) primary key);"); > +box.space.T6:insert({11}) > +test_run:cmd("restart server default") > +box.space.T6:insert({11}) > + > +box.space.T6:drop() > +box.func.MYFUNC:drop() > -- > 2.22.1 >
parent reply other threads:[~2019-09-10 21:22 UTC|newest] Thread overview: expand[flat|nested] mbox.gz Atom feed [parent not found: <33fa81dad5745292133063d39a3c20144718bb60.1567496399.git.kshcherbatov@tarantool.org>]
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=20190910212237.GA27128@tarantool.org \ --to=korablev@tarantool.org \ --cc=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: disallow ck using non-persistent function' \ /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