From: imeevma@tarantool.org To: korablev@tarantool.org Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] [PATCH v3 1/4] sql: remove unnecessary AUTOINCREMENT ID generation Date: Thu, 11 Jul 2019 11:22:30 +0300 [thread overview] Message-ID: <89267d98f7f336a5c76fd72e3888b83954b32f45.1562832978.git.imeevma@gmail.com> (raw) In-Reply-To: <cover.1562832978.git.imeevma@gmail.com> Hi! Thank you for review. My answers and new patch below. There won't be diff since I just removed tests. On 7/8/19 9:54 PM, n.pettik wrote: > > >> On 4 Jul 2019, at 14:42, imeevma@tarantool.org wrote: >> >> Currently, if we perform something like >> CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT); >> INSERT INTO t(a) VALUES (NULL); >> >> we generate a new identifier in a special way. > > Describe this “special” way. > Fixed commit-message. >> src/box/sql/insert.c | 5 +---- >> test/sql/gh-2981-check-autoinc.result | 32 ++++++++++++++++++++++++++++++++ >> test/sql/gh-2981-check-autoinc.test.lua | 11 ++++++++++- >> 3 files changed, 43 insertions(+), 5 deletions(-) >> >> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c >> index b353148..d2b4e17 100644 >> --- a/src/box/sql/insert.c >> +++ b/src/box/sql/insert.c >> @@ -628,10 +628,7 @@ sqlInsert(Parse * pParse, /* Parser context */ >> if (j < 0 || nColumn == 0 >> || (pColumn && j >= pColumn->nId)) { >> if (i == (int) autoinc_fieldno) { >> - sqlVdbeAddOp2(v, >> - OP_NextAutoincValue, >> - space->def->id, >> - iRegStore); > > Why didn’t you delete OP_NextAutioncValue? > I changed it functionality in the next patch. >> + sqlVdbeAddOp2(v, OP_Null, 0, iRegStore); >> continue; >> } >> struct Expr *dflt = NULL; >> >> diff --git a/test/sql/gh-2981-check-autoinc.test.lua b/test/sql/gh-2981-check-autoinc.test.lua >> index 0eb8f73..ac5624e 100644 >> --- a/test/sql/gh-2981-check-autoinc.test.lua >> +++ b/test/sql/gh-2981-check-autoinc.test.lua >> @@ -7,6 +7,8 @@ box.cfg{} >> box.execute("CREATE TABLE t1 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));"); >> box.execute("CREATE TABLE t2 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19 AND s1 <> 25));"); >> box.execute("CREATE TABLE t3 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));”); >> +box.execute("CREATE TABLE t4 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 < 10));"); >> +box.execute("CREATE TABLE t5 (s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19));”); > > Add explanation to these tests.. What do they verify? > I build master branch and they are passed as well. > I deleted the tests. They don't really show anything as they should, and this was described in the commit message. They would be useful if you run them before #3691 was pushed. New patch: From 89267d98f7f336a5c76fd72e3888b83954b32f45 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Wed, 3 Jul 2019 14:05:11 +0300 Subject: [PATCH] sql: remove unnecessary AUTOINCREMENT ID generation Currently, if we perform something like CREATE TABLE t1 ( s1 INTEGER PRIMARY KEY AUTOINCREMENT, s2 INTEGER, CHECK (s1 <> 19) ); INSERT INTO t1 VALUES (18, NULL); INSERT INTO t1 (s2) VALUES (NULL); we generate a new identifier in VDBE, but in any other case we generate it in BOX. That was needed since the CHECK did not work properly. This is not necessary now, because CHECK was moved to BOX due to issue #3691. After this patch all new identifiers will be generated in BOX. Part of #4188 diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index b353148..d2b4e17 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -628,10 +628,7 @@ sqlInsert(Parse * pParse, /* Parser context */ if (j < 0 || nColumn == 0 || (pColumn && j >= pColumn->nId)) { if (i == (int) autoinc_fieldno) { - sqlVdbeAddOp2(v, - OP_NextAutoincValue, - space->def->id, - iRegStore); + sqlVdbeAddOp2(v, OP_Null, 0, iRegStore); continue; } struct Expr *dflt = NULL;
next prev parent reply other threads:[~2019-07-11 8:22 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-11 8:22 [tarantool-patches] [PATCH v3 0/4] sql: do not show IDs generated by trigger imeevma 2019-07-11 8:22 ` imeevma [this message] 2019-07-15 17:50 ` [tarantool-patches] Re: [PATCH v3 1/4] sql: remove unnecessary AUTOINCREMENT ID generation n.pettik 2019-07-11 8:22 ` [tarantool-patches] [PATCH v3 2/4] sql: do not show IDs generated by trigger imeevma 2019-07-15 17:50 ` [tarantool-patches] " n.pettik 2019-07-11 8:22 ` [tarantool-patches] [PATCH v3 3/4] sql: remove VDBE from TXN imeevma 2019-07-11 8:22 ` [tarantool-patches] [PATCH v3 4/4] sql: do not show generated IDs if INSERT failed imeevma 2019-07-11 8:52 ` [tarantool-patches] " Mergen Imeev 2019-07-15 17:59 ` 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=89267d98f7f336a5c76fd72e3888b83954b32f45.1562832978.git.imeevma@gmail.com \ --to=imeevma@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH v3 1/4] sql: remove unnecessary AUTOINCREMENT ID generation' \ /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