From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Sat, 21 Jul 2018 23:59:48 +0300 From: Konstantin Osipov Subject: Re: [PATCH 5/6] space: call before_replace trigger even if space has no indexes Message-ID: <20180721205948.GA6906@chai> References: <35469846b9a1e53af007f600a120c716478d253b.1532107326.git.vdavydov.dev@gmail.com> <20180720183909.GF4827@chai> <20180721122652.rll2tshmcsbhzsfn@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180721122652.rll2tshmcsbhzsfn@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [18/07/21 23:52]: > On Fri, Jul 20, 2018 at 09:39:09PM +0300, Konstantin Osipov wrote: > > * Vladimir Davydov [18/07/20 20:45]: > > > Needed for blackhole spaces (see the next patch), which don't support > > > indexes per se, but still may have a before_replace trigger installed. > > > > Why do you need to enable before replace triggers for blackhole? > > Why not? before_replace can be installed on a blackhole space so > I guess it should work, otherwise it may raise questions. > > > What's wrong with on_replace triggers? > > Nothing wrong at all. They work as well. OK, maybe I'm a performance junkie, but I saw you had to pessimize before_replace to add support for blackhole, and thought for a minute that for the task at hand you could get by with on_replace(). Now I realize your patch is off the hot path, before_replace is expensive because it makes a lookup, not because there's a few branches here and there. Thanks, it's OK to push this patch as well then. I assume you will be adding a test case for space-with-no-indexes in a later patch? -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov