From: Imeev Mergen <imeevma@tarantool.org> To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: do not show additional generated ids Date: Tue, 25 Jun 2019 17:09:31 +0300 [thread overview] Message-ID: <7af7c576-0555-558b-f905-bcb7d98afb78@tarantool.org> (raw) In-Reply-To: <14D3EEAF-2501-4BDC-82B6-C0BD3729C57A@tarantool.org> Hi! Thank you for review! My answers below. On 6/7/19 3:38 AM, n.pettik wrote: >> On 29 May 2019, at 20:09, imeevma@tarantool.org wrote: >> >> Currently, if INSERT is executed by a trigger, the new generated >> identifiers will be stored in the VDBE. This is wrong, and this >> patch fixes it. > Why is it so? Currently, we are not able to divide the received IDS into those that belong to the mentioned table and those that do not belong. >> Only identifiers generated during INSERT run by >> the user will be saved. > Firstly, I truly do not understand why this behaviour > is considered to be buggy. I don’t see discussion or > bug description. Now this discussion can be viewed on the dev mailing list. > Secondly, I do not understand what happens in this patch. > Could you please explain these changes and argue why > they are needed? Changes in this patch: 1) At first I removed changes made in patch #2981 since it isn't necessary now. Due to this, all the generation of new identifiers is now performed in BOX. 2) I removed VDBE from TXN. So now sequence_next() does not save the generated IDs in VDBE. 3) I changed the way OP_NextAutoincValue works. Now it is executed when there is an insert in the field with AUTOINCREMENT. If NULL was inserted it receives the last generated ID using the new function sequence_get_value(), and stores it in VDBE. Otherwise it is a no-op. 4) I added an extra argument to the sqlInsert() function, which makes it clear that the insertion was done in a trigger. My point about these changes: 1 - I think it should be done anyway. 2 - It doesn't look like VDBE should be stored in TXN. 3 - I think we can avoid creating the function sequence_get_value(), but then we should think about how to store the last generated ID. 4 - I think this change has a pretty clear purpose.
next prev parent reply other threads:[~2019-06-25 14:09 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-29 17:09 [tarantool-patches] " imeevma 2019-06-07 0:38 ` [tarantool-patches] " n.pettik 2019-06-25 14:09 ` Imeev Mergen [this message] 2019-07-03 0:25 ` n.pettik
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=7af7c576-0555-558b-f905-bcb7d98afb78@tarantool.org \ --to=imeevma@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: do not show additional generated ids' \ /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