From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 dev.tarantool.org (Postfix) with ESMTPS id B2CB34696C3 for ; Tue, 3 Mar 2020 13:13:33 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <980e9649-2e7c-424c-7d75-1945cb24d587@tarantool.org> Date: Tue, 3 Mar 2020 13:13:31 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <8681EE81-5099-4112-8C62-C63E873535E7@tarantool.org> References: <20200109101502.4158-1-roman.habibov@tarantool.org> <1fc992ec-c0b8-320e-699d-bfa8047c9833@tarantool.org> <1509E852-025B-4147-9DFE-FA4202C5C3A0@tarantool.org> <980e9649-2e7c-424c-7d75-1945cb24d587@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 3/3] sql: support constraint drop List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. > On Feb 29, 2020, at 18:32, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! >=20 >>>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >>>> index d9bf8de91..76ad79350 100644 >>>> --- a/src/box/sql/build.c >>>> +++ b/src/box/sql/build.c >>>> @@ -2052,35 +2052,78 @@ fk_constraint_change_defer_mode(struct = Parse *parse_context, bool is_deferred) >>>> is_deferred; >>>> } >>>>=20 >>>> +/** >>>> + * Emit code to drop the entry from _index or _ck_contstraint or >>>> + * _fk_constraint space corresponding with the constraint type. >>>> + */ >>>> void >>>> -sql_drop_foreign_key(struct Parse *parse_context) >>>> +sql_drop_constraint(struct Parse *parse_context) >>>> { >>>> - struct drop_entity_def *drop_def =3D = &parse_context->drop_fk_def.base; >>>> - assert(drop_def->base.entity_type =3D=3D ENTITY_TYPE_FK); >>>> + struct drop_entity_def *drop_def =3D >>>> + &parse_context->drop_constraint_def.base; >>>> + assert(drop_def->base.entity_type =3D=3D = ENTITY_TYPE_CONSTRAINT); >>>> assert(drop_def->base.alter_action =3D=3D ALTER_ACTION_DROP); >>>> const char *table_name =3D = drop_def->base.entity_name->a[0].zName; >>>> assert(table_name !=3D NULL); >>>> - struct space *child =3D space_by_name(table_name); >>>> - if (child =3D=3D NULL) { >>>> + struct space *space =3D space_by_name(table_name); >>>> + if (space =3D=3D NULL) { >>>> diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); >>>> parse_context->is_aborted =3D true; >>>> return; >>>> } >>>> - char *constraint_name =3D >>>> - sql_name_from_token(parse_context->db, &drop_def->name); >>>> - if (constraint_name =3D=3D NULL) { >>>> + char *name =3D sql_name_from_token(parse_context->db, = &drop_def->name); >>>> + if (name =3D=3D NULL) { >>>> + parse_context->is_aborted =3D true; >>>> + return; >>>> + } >>>> + struct constraint_id *id =3D space_find_constraint_id(space, = name); >>>> + if (id =3D=3D NULL) { >>>=20 >>> 2. We perhaps need to do that at runtime - search in fk, ck, and = index >>> spaces, or somehow in this hash table. Because otherwise we rely on >>> having struct space object, which in theory won't always be the = case. >>> which in theory won't always be the case. >> Why? Can you, please, explain? >=20 > Remote client nodes do not have any schema objects. If a request > requires struct space *, it can't be compiled on the client. And > this is where we are going to. >=20 >>> However, not in scope of this patch maybe. Because anyway we have >>> struct space used in lots of other places. And I don't know a = general >>> solution how to get rid of all of them. Yet. >> Do you mean to avoid struct space usage within build.c? >=20 > Yes, I want us to avoid any schema objects usage where possible. >=20 >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index 00877b7d8..17fd07f78 100644 >> --- a/src/box/sql/build.c >> +++ b/src/box/sql/build.c >> @@ -1519,7 +1517,8 @@ vdbe_emit_fk_constraint_drop(struct Parse = *parse_context, char *constraint_name, >=20 > constraint_name can become 'const' now. Done. >> struct Vdbe *vdbe =3D sqlGetVdbe(parse_context); >> assert(vdbe !=3D NULL); >> int key_reg =3D sqlGetTempRange(parse_context, 3); >> - sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg, 0, constraint_name, >> + const char *name_copy =3D sqlDbStrDup(parse_context->db, = constraint_name); >> + sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg, 0, name_copy, >> P4_DYNAMIC); >> sqlVdbeAddOp2(vdbe, OP_Integer, child_def->id, key_reg + 1); >> const char *error_msg =3D