From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 23 Oct 2018 11:32:06 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 8/8] tuple: zap tuple_extra Message-ID: <20181023083206.zbdpbbp4oaesezmd@esperanza> References: <137ff9cb723921ebcaa4af3455b2466866f17047.1539539421.git.vdavydov.dev@gmail.com> <20181023074211.GI9849@chai> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181023074211.GI9849@chai> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: On Tue, Oct 23, 2018 at 10:42:11AM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [18/10/15 10:15]: > > Actually, the whole idea of tuple_extra() is rather crooked: why > > would we need it if we can inherit struct tuple instead, as we do > > in case of memtx_tuple and vy_stmt? Accessing an inherited struct > > is much more convenient than using tuple_extra(). > > Because you don't want to inherit a tuple to store last access > time, or some information specific to functional index or row id. > That was the idea at least. But I do want to inherit memtx_tuple for storing atime. It would look much cleaner IMO. Regarding functional indexes, in my understanding we don't want to store any extra information in struct tuple, because that wouldn't work in case of vinyl. Extra info should be stored right in tuple data IMHO or may be we could implement functional indexes as shadow spaces, dunno... This remains to be decided though. Regarding row id. Again, wouldn't work in case of vinyl. Should be stored in a hidden field rather than in tuple extra. I agree that there may be problems with inheritance if we ever want to store atime and something else (say ctime) for some spaces and only atime or only ctime for others. But current implementation of tuple extra wouldn't suit for this either and would need a redesign anyway. > > This patch and the entire patch set is OK to push, when we get to > implementing the above mentioned features we can consider putting > it back in again. > > > > So this patch gets rid of tuple_extra(). To do that, it partially > > reverts the following commits: