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 84E6B248EA for ; Thu, 4 Jul 2019 06:49:48 -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 3IHi47pMy7L1 for ; Thu, 4 Jul 2019 06:49:48 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 44CD320D93 for ; Thu, 4 Jul 2019 06:49:48 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v3 3/3] sql: use common registers instead of temp. for constraints data Date: Thu, 4 Jul 2019 13:49:46 +0300 Message-Id: <75fdfbc986b1aae8a476017f3c417c25c357dd65.1562235700.git.imeevma@gmail.com> In-Reply-To: References: 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 Thank you for review! New patch below. On 7/3/19 2:27 AM, n.pettik wrote: > > >> On 28 Jun 2019, at 15:07, imeevma@tarantool.org wrote: >> >> Prior to this patch, the data needed to create the constraints in >> VDBE was stored in the temporary registers of the parser. Since >> this data is necessary for creating destructors, it was >> transferred to standard registers. > > sql: use common registers instead of temp. for constraints data > > Prior to this patch, data needed to form tuple to be inserted to > _fk_constraint and _ck_constraint system spaces (to create corresponding > constraints) was stored in the range of temporary register. After > insertion, temporary registers are released. On the other hand, this > data is required for providing clean-up in case of creation fail (i.e. > removing already created constraints within one CREATE TABLE statement). > Hence, instead of using temporary registers let's use ordinary ones. > > Closes #4183 Fixed. >From 75fdfbc986b1aae8a476017f3c417c25c357dd65 Mon Sep 17 00:00:00 2001 Date: Thu, 27 Jun 2019 17:57:28 +0300 Subject: [PATCH] sql: use common registers instead of temp. for constraints data Prior to this patch, data needed to form tuple to be inserted to _fk_constraint and _ck_constraint system spaces (to create corresponding constraints) was stored in the range of temporary register. After insertion, temporary registers are released. On the other hand, this data is required for providing clean-up in case of creation fail (i.e. removing already created constraints within one CREATE TABLE statement). Hence, instead of using temporary registers let's use ordinary ones. Closes #4183 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 0dd2906..1c4b4f7 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1078,7 +1078,12 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, struct sql *db = parser->db; struct Vdbe *v = sqlGetVdbe(parser); assert(v != NULL); - int ck_constraint_reg = sqlGetTempRange(parser, 6); + /* + * Occupy registers for 5 fields: each member in + * _ck_constraint space plus one for final msgpack tuple. + */ + int ck_constraint_reg = parser->nMem + 1; + parser->nMem += 6; sqlVdbeAddOp2(v, OP_SCopy, reg_space_id, ck_constraint_reg); sqlVdbeAddOp4(v, OP_String8, 0, ck_constraint_reg + 1, 0, sqlDbStrDup(db, ck_def->name), P4_DYNAMIC); @@ -1102,7 +1107,6 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, save_record(parser, BOX_CK_CONSTRAINT_ID, ck_constraint_reg, 2, v->nOp - 1, true); VdbeComment((v, "Create CK constraint %s", ck_def->name)); - sqlReleaseTempRange(parser, ck_constraint_reg, 5); } /** @@ -1121,10 +1125,11 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, struct Vdbe *vdbe = sqlGetVdbe(parse_context); assert(vdbe != NULL); /* - * Occupy registers for 8 fields: each member in - * _constraint space plus one for final msgpack tuple. + * Occupy registers for 9 fields: each member in + * _fk_constraint space plus one for final msgpack tuple. */ - int constr_tuple_reg = sqlGetTempRange(parse_context, 10); + int constr_tuple_reg = parse_context->nMem + 1; + parse_context->nMem += 10; char *name_copy = sqlDbStrDup(parse_context->db, fk->name); if (name_copy == NULL) return; @@ -1211,7 +1216,6 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, } save_record(parse_context, BOX_FK_CONSTRAINT_ID, constr_tuple_reg, 2, vdbe->nOp - 1, true); - sqlReleaseTempRange(parse_context, constr_tuple_reg, 10); return; error: parse_context->is_aborted = true; diff --git a/test/sql/clear.result b/test/sql/clear.result index 2034d82..2fb1761 100644 --- a/test/sql/clear.result +++ b/test/sql/clear.result @@ -198,3 +198,23 @@ box.space._fk_constraint:select() --- - [] ... +-- +-- Make sure that keys for tuples inserted into system spaces were +-- not stored in temporary cells. +-- +box.execute("CREATE TABLE t3(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT ck1 CHECK(id < 0));") +--- +- error: Constraint CK1 already exists +... +box.space.t1 +--- +- null +... +box.space._ck_constraint:select() +--- +- [] +... +box.space._fk_constraint:select() +--- +- [] +... diff --git a/test/sql/clear.test.lua b/test/sql/clear.test.lua index 7ed18a4..4c58767 100644 --- a/test/sql/clear.test.lua +++ b/test/sql/clear.test.lua @@ -43,3 +43,12 @@ box.space._ck_constraint:select() box.execute("CREATE TABLE t2(id INT PRIMARY KEY, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2);") box.space.t2 box.space._fk_constraint:select() + +-- +-- Make sure that keys for tuples inserted into system spaces were +-- not stored in temporary cells. +-- +box.execute("CREATE TABLE t3(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT ck1 CHECK(id < 0));") +box.space.t1 +box.space._ck_constraint:select() +box.space._fk_constraint:select()