From: Roman Khabibov <roman.habibov@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 3/3] sql: support constraint drop Date: Tue, 3 Mar 2020 13:13:31 +0300 [thread overview] Message-ID: <8681EE81-5099-4112-8C62-C63E873535E7@tarantool.org> (raw) In-Reply-To: <980e9649-2e7c-424c-7d75-1945cb24d587@tarantool.org> Hi! Thanks for the review. > On Feb 29, 2020, at 18:32, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > > 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. Done. >> 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 =
next prev parent reply other threads:[~2020-03-03 10:13 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-09 10:15 [Tarantool-patches] [PATCH] " Roman Khabibov 2020-01-13 17:00 ` Nikita Pettik 2020-01-24 14:21 ` [Tarantool-patches] [PATCH 2/2] " Roman Khabibov 2020-01-28 17:39 ` Nikita Pettik 2020-02-01 17:36 ` Roman Khabibov 2020-02-11 16:56 ` Nikita Pettik 2020-02-16 10:24 ` Roman Khabibov 2020-02-20 19:55 ` Nikita Pettik 2020-02-20 23:09 ` [Tarantool-patches] [PATCH] " Vladislav Shpilevoy 2020-02-20 23:36 ` Nikita Pettik 2020-02-29 12:47 ` [Tarantool-patches] [PATCH v2 3/3] " Roman Khabibov 2020-02-29 15:32 ` Vladislav Shpilevoy 2020-03-03 10:13 ` Roman Khabibov [this message] 2020-03-03 10:12 [Tarantool-patches] [PATCH v2 0/3] Add ability to drop constraints Roman Khabibov 2020-03-03 10:12 ` [Tarantool-patches] [PATCH v2 3/3] sql: support constraint drop Roman Khabibov
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=8681EE81-5099-4112-8C62-C63E873535E7@tarantool.org \ --to=roman.habibov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 3/3] sql: support constraint drop' \ /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