[tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module

[Vladimir Davydov]

On Thu, Apr 04, 2019 at 02:46:22PM +0300, Alexander Turenko wrote:
> > > >> Probably, we should reuse tuple_validate() for checking a tuple against
> > > >> a key_def so as not to implement the same code again.
> > > 
> > > Unfortunately tuple_validate() is designed for format validation while we don't
> > > have format here and I don't like create it for validation event in case of error.
> > 
> > Creating a format on each call to 'compare' is prohibitive from
> > performance pov, you're right. However, we could attach a format to
> > Lua's key_def object. This would also speed up comparisons, as tuples
> 
> So this module will not more be relatively simple layer that exposes
> box/key_def into Lua and this again suggests me that the name key_def
> maybe is not good choice.
> 
> > created for this format would have fieldmap.
> 
> Note: It will speed up comparisons only in case when a user give a lua

Yeah, right. So speeding up comparisons isn't an iron-clad argument
for attaching a format to Lua key_def object. May be not worth the
complexity it will introduce.

> table as input, not when it give a tuple. However if a user give a tuple
> it seems that it is in (s)he responsibility to think about the format.
> 
> And, while we're here, maybe it worth to provide a function like
> box.tuple.new(), but with key_def instance as an argument to allow a
> user to create tuples with necessary formats (to speed up comparisons
> and other fields accesses)?

No, I don't think so: the API wouldn't be generic enough, because one
might want to compare the same tuple using different key definitions.

> 
> > However, what I don't like is that in case a tuple is created from a Lua
> > table, you'll have to access all fields twice - on validation and
> > comparison - which is costly without a fieldmap. One more argument for
> > attaching a format to Lua's key_def object.
> 
> I think we can add *_unchecked() function variants like in
> msgpack/msgpackffi if the peformance will be really a problem and
> someone will request for them.

I don't think that _unchecked (i.e. crash on malformed input) is a good
idea, especially in this case.