Re: [Tarantool-patches] [PATCH 13/14] WIP: module api: expose box_tuple_extract_key_ex()

[Alexander Turenko <alexander.turenko@tarantool.org>]

On Sat, Sep 26, 2020 at 12:58:27AM +0200, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> Or instead of _ex you could make it a method of key_def and
> call it box_key_def_extract_key(...). IMO that would be more
> appropriate as a key_def method.

Since more questions about functions naming arose, I put my thoughts to
[1].

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/019989.html

> > +/** \cond public */
> > +
> >  /**
> >   * A special value of multikey index that means that the key
> >   * definition is not multikey and no indirection is expected.
> >   */
> >  enum { MULTIKEY_NONE = -1 };
> 
> 1. Why didn't you add a new BOX_KEY_DEF_MULTIKEY_NONE constant?
> I thought we don't want to expose any internal definitions.
> 
> Also you could specify in the comment that pass -1 if don't want
> to use multikey. Without a enum. I think it would be totally
> fine.

I agree. Will remove it in the next patchset version.

> 
> > diff --git a/src/box/key_def_api.h b/src/box/key_def_api.h
> > index 0cef7d1ea..28fcc32da 100644
> > --- a/src/box/key_def_api.h
> > +++ b/src/box/key_def_api.h
> > @@ -243,6 +243,26 @@ box_tuple_compare_with_key(box_tuple_t *tuple_a, const char *key_b,
> >  API_EXPORT box_key_def_t *
> >  box_key_def_merge(const box_key_def_t *first, const box_key_def_t *second);
> >  
> > +/**
> > + * Extract key from tuple by given key definition and return
> > + * buffer allocated on the box region with this key.
> > + * @sa <box_region_truncate>().
> > + *
> > + * This function has O(n) complexity, where n is the number of key
> > + * parts.
> > + *
> > + * @param tuple - tuple from which need to extract key
> 
> 2. Lets be consistent and not use '-' between '@param name'
> and the parameter description. In addition to using capital
> letter in the beginning and dots in the end. Sorry for OCD.

Okay.