From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
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 3F5F1469719
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 21 Feb 2020 02:36:09 +0300 (MSK)
Date: Fri, 21 Feb 2020 02:36:07 +0300
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200220233607.GB51816@tarantool.org>
References: <20200109101502.4158-1-roman.habibov@tarantool.org>
 <1fc992ec-c0b8-320e-699d-bfa8047c9833@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1fc992ec-c0b8-320e-699d-bfa8047c9833@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH] sql: support constraint drop
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

On 21 Feb 00:09, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > -	vdbe_emit_fk_constraint_drop(parse_context, constraint_name,
> > -				     child->def);
> > -	/*
> > -	 * We account changes to row count only if drop of
> > -	 * foreign keys take place in a separate
> > -	 * ALTER TABLE DROP CONSTRAINT statement, since whole
> > -	 * DROP TABLE always returns 1 (one) as a row count.
> > -	 */
> >  	struct Vdbe *v = sqlGetVdbe(parse_context);
> > +	assert(v != NULL);
> > +	assert(id->type < constraint_type_MAX);
> > +	switch (id->type) {
> > +	case CONSTRAINT_TYPE_PK:
> > +	case CONSTRAINT_TYPE_UNIQUE: {
> > +		uint32_t index_id = box_index_id_by_name(space->def->id, name,
> > +							 strlen(name));
> 
> 3. This is definitely not ok. It is not just looking at space *, it is a
> select during parsing. _index has a unique index by space id and name, so
> you can emit a deletion opcode without learning an index id. It shouldn't
> be hard.

Fair note. Then additional request from me: let's remove
box_index_id_by_name() from sql_drop_index() and pragma_index_info()
as well (in a separate patch ofc).