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 4054725AE6 for ; Thu, 11 Jul 2019 04:22:33 -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 VpLqUvLGmZx8 for ; Thu, 11 Jul 2019 04:22:33 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 F3C8C25AE5 for ; Thu, 11 Jul 2019 04:22:32 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v3 1/4] sql: remove unnecessary AUTOINCREMENT ID generation Date: Thu, 11 Jul 2019 11:22:30 +0300 Message-Id: <89267d98f7f336a5c76fd72e3888b83954b32f45.1562832978.git.imeevma@gmail.com> MIME-Version: 1.0 In-Reply-To: References: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit 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: korablev@tarantool.org Cc: tarantool-patches@freelists.org 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 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;