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 84FA0280AD for ; Mon, 6 Aug 2018 19:43:46 -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 VVOm_5vrBYm6 for ; Mon, 6 Aug 2018 19:43:46 -0400 (EDT) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 E4DC5280A2 for ; Mon, 6 Aug 2018 19:43:45 -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 3/5] sql: introduce ADD CONSTRAINT statement From: "n.pettik" In-Reply-To: <17680404-70f3-93bb-1f7c-024ff45a3530@tarantool.org> Date: Tue, 7 Aug 2018 02:43:38 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1EF951A7-A7E7-4EC3-BA78-DB3A5E66A461@tarantool.org> References: <092e07d5399d662cd38b755a0403b11cc4dde2a1.1531443603.git.korablev@tarantool.org> <647dcc4b-a2c4-5cc8-c81c-2dbb66ce18ed@tarantool.org> <9B5CA4F9-A6DB-46FF-BE86-CC3900D22D97@tarantool.org> <17680404-70f3-93bb-1f7c-024ff45a3530@tarantool.org> 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: Vladislav Shpilevoy Also I=E2=80=99ve rebased patch-set on fresh 2.0. >>>> + /* Delete all child FK constraints. */ >>>> + struct fkey *child_fk; >>>> + rlist_foreach_entry (child_fk, &space->child_fkey, child_link) { >>>> + const char *fk_name_dup =3D sqlite3DbStrDup(v->db, >>>> + = child_fk->def->name); >>>> + if (fk_name_dup =3D=3D NULL) >>>> + return; >>>> + vdbe_emit_fkey_drop(parse_context, fk_name_dup, = space_id); >>>=20 >>> 5. Can we use P4_STATIC and do not duplicate? >> No, unfortunately we can=E2=80=99t. P4_STATIC attribute is usable = only when the same >> memory is used twice during VDBE execution. STATIC attribute prevents = from >> memory releasing (even at the end of execution AFAIK). If we released = memory >> right after generating opcode (or at the end of parsing, it doesn=E2=80= =99t matter), we will >> probably get corrupted memory. >=20 > How can child_fk->def->name be freed before VDBE execution? It is drop = table > code generation, so here the child_fk exists out of the parser context = in a > space from the space cache. >=20 >>> I want to say, that if we >>> do not duplicate the memory here, then only 2 situations are = possible: >>> 1) Until the VDBE is executed nothing is happened, the memory is ok = and >>> can be used. >> Can be used, but wouldn=E2=80=99t be freed (or I misunderstand what = you initially meant). >=20 > Yes, it will not be freed because it will not be duplicated. Free of = the > original name together with the child_fk will occur inside alter.cc. = Vdbe > after drop of the space will not touch this freed memory. >=20 > But do not mind. I see, that vdbe_emit_fkey_drop is used both to = malloced Yep, I meant exactly this usage. >> diff --git a/test/sql-tap/fkey2.test.lua = b/test/sql-tap/fkey2.test.lua >> index 062597e9b..7cb315fcc 100755 >> --- a/test/sql-tap/fkey2.test.lua >> +++ b/test/sql-tap/fkey2.test.lua >> @@ -755,10 +753,13 @@ test:do_catchsql_test( >> DROP TABLE IF EXISTS p; >> CREATE TABLE p(a, b, PRIMARY KEY(a, b)); >> CREATE TABLE c(x PRIMARY KEY REFERENCES p); >> +<<<<<<< HEAD >> INSERT INTO c DEFAULT VALUES; >> +=3D=3D=3D=3D=3D=3D=3D >> +>>>>>>> 323e96ee2... sql: introduce ADD CONSTRAINT statement >=20 > 1. ??? Oops, sorry, =E2=80=98haste makes waste=E2=80=99... > 2. Please, do not use errinj in regular tests. Only > in release-disabled. Errinj structures and code are > defined to nothing in C when NDEBUG is set, so they > do not work. Forgot about that. So simply moved these tests to sql/erring.test.lua