From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 AE7F74696C4 for ; Sat, 29 Feb 2020 18:32:03 +0300 (MSK) References: <20200109101502.4158-1-roman.habibov@tarantool.org> <1fc992ec-c0b8-320e-699d-bfa8047c9833@tarantool.org> <1509E852-025B-4147-9DFE-FA4202C5C3A0@tarantool.org> From: Vladislav Shpilevoy Message-ID: <980e9649-2e7c-424c-7d75-1945cb24d587@tarantool.org> Date: Sat, 29 Feb 2020 16:32:01 +0100 MIME-Version: 1.0 In-Reply-To: <1509E852-025B-4147-9DFE-FA4202C5C3A0@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! >>> 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; >>> } >>> >>> +/** >>> + * 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 = &parse_context->drop_fk_def.base; >>> - assert(drop_def->base.entity_type == ENTITY_TYPE_FK); >>> + struct drop_entity_def *drop_def = >>> + &parse_context->drop_constraint_def.base; >>> + assert(drop_def->base.entity_type == ENTITY_TYPE_CONSTRAINT); >>> assert(drop_def->base.alter_action == ALTER_ACTION_DROP); >>> const char *table_name = drop_def->base.entity_name->a[0].zName; >>> assert(table_name != NULL); >>> - struct space *child = space_by_name(table_name); >>> - if (child == NULL) { >>> + struct space *space = space_by_name(table_name); >>> + if (space == NULL) { >>> diag_set(ClientError, ER_NO_SUCH_SPACE, table_name); >>> parse_context->is_aborted = true; >>> return; >>> } >>> - char *constraint_name = >>> - sql_name_from_token(parse_context->db, &drop_def->name); >>> - if (constraint_name == NULL) { >>> + char *name = sql_name_from_token(parse_context->db, &drop_def->name); >>> + if (name == NULL) { >>> + parse_context->is_aborted = true; >>> + return; >>> + } >>> + struct constraint_id *id = space_find_constraint_id(space, name); >>> + if (id == NULL) { >> >> 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? 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. >> 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? Yes, I want us to avoid any schema objects usage where possible. > 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, constraint_name can become 'const' now. > struct Vdbe *vdbe = sqlGetVdbe(parse_context); > assert(vdbe != NULL); > int key_reg = sqlGetTempRange(parse_context, 3); > - sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg, 0, constraint_name, > + const char *name_copy = 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 =