From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 3/5] sql: introduce ADD CONSTRAINT statement Date: Tue, 7 Aug 2018 02:43:38 +0300 [thread overview] Message-ID: <1EF951A7-A7E7-4EC3-BA78-DB3A5E66A461@tarantool.org> (raw) In-Reply-To: <17680404-70f3-93bb-1f7c-024ff45a3530@tarantool.org> Also I’ve 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 = sqlite3DbStrDup(v->db, >>>> + child_fk->def->name); >>>> + if (fk_name_dup == NULL) >>>> + return; >>>> + vdbe_emit_fkey_drop(parse_context, fk_name_dup, space_id); >>> >>> 5. Can we use P4_STATIC and do not duplicate? >> No, unfortunately we can’t. 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’t matter), we will >> probably get corrupted memory. > > 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. > >>> 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’t be freed (or I misunderstand what you initially meant). > > 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. > > 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; >> +======= >> +>>>>>>> 323e96ee2... sql: introduce ADD CONSTRAINT statement > > 1. ??? Oops, sorry, ‘haste makes waste’... > 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
next prev parent reply other threads:[~2018-08-06 23:43 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-13 2:04 [tarantool-patches] [PATCH 0/5] Move FK constraints to server Nikita Pettik 2018-07-13 2:04 ` [tarantool-patches] [PATCH 1/5] sql: prohibit creation of FK on unexisting tables Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:27 ` n.pettik 2018-07-13 2:04 ` [tarantool-patches] [PATCH 2/5] schema: add new system space for FK constraints Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:28 ` n.pettik 2018-08-06 18:24 ` Vladislav Shpilevoy 2018-07-13 2:04 ` [tarantool-patches] [PATCH 3/5] sql: introduce ADD CONSTRAINT statement Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:28 ` n.pettik 2018-08-06 18:24 ` Vladislav Shpilevoy 2018-08-06 23:43 ` n.pettik [this message] 2018-07-13 2:04 ` [tarantool-patches] [PATCH 4/5] sql: display error on FK creation and drop failure Nikita Pettik 2018-07-17 21:04 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:11 ` Vladislav Shpilevoy 2018-07-13 2:04 ` [tarantool-patches] [PATCH 5/5] sql: remove SQLITE_OMIT_FOREIGN_KEY define guard Nikita Pettik 2018-07-17 21:04 ` [tarantool-patches] Re: [PATCH 0/5] Move FK constraints to server Vladislav Shpilevoy 2018-08-07 14:57 ` 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=1EF951A7-A7E7-4EC3-BA78-DB3A5E66A461@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH 3/5] sql: introduce ADD CONSTRAINT statement' \ /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