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 9C3CF25003 for ; Wed, 24 Jul 2019 18:25:03 -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 XbfGBdGRomTJ for ; Wed, 24 Jul 2019 18:25:03 -0400 (EDT) Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 544E72315F for ; Wed, 24 Jul 2019 18:25:03 -0400 (EDT) Received: by smtp47.i.mail.ru with esmtpa (envelope-from ) id 1hqPgz-0001a7-AW for tarantool-patches@freelists.org; Thu, 25 Jul 2019 01:25:01 +0300 Date: Thu, 25 Jul 2019 01:25:01 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 1/3] vinyl: use update_def index method to update vy_lsm on ddl Message-ID: <20190724222500.GA13150@atlas> References: <20a1aafbfba6068f17a07b01e4c829db0d429fe0.1563895921.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20a1aafbfba6068f17a07b01e4c829db0d429fe0.1563895921.git.vdavydov.dev@gmail.com> 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: tarantool-patches@freelists.org * Vladimir Davydov [19/07/23 18:49]: > When an index definition is modified by DDL in such a way that doesn't > require index rebuild (e.g. a key part type is changed from unsigned to > integer), we move the index from the old space container to the new one, > see ModifyIndex. In case of Vinyl we also need to update key definitions > and options stored in vy_lsm. We do that in swap_index space method, but > this isn't entirely correct, as this method is also called when an index > is moved without any modifications, see MoveIndex. Let's do this from > update_def index method instead, which seems to be more suitable. > > The only reason this code lives in swap_index space method now is that > we didn't have update_def index method when this functionality was > introduced in the first place. First of all, it is extremely annoying that instead of testing this properly despite my humble, repeated requests we waited for these bugs to surface and now address them with urgent patches. I see if I could find anything wrong with this patch but I can't. > +static struct key_def * > +key_def_do_copy(struct key_def *res, const struct key_def *src, size_t sz) I personally prefer _impl to do_. -- Konstantin Osipov, Moscow, Russia