Re: [tarantool-patches] [PATCH v3 4/4] box: introduce functional indexes in memtx

[Konstantin Osipov <kostja@tarantool.org>]

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/07/17 09:39]:
> +++ b/src/box/CMakeLists.txt
> @@ -51,8 +51,9 @@ add_library(tuple STATIC
>      coll_id_cache.c
>      field_def.c
>      opt_def.c
> +    functional_extractor.c

since we started to use func_ and not function_ and functional_ as
an abbreviation, let's be consistent and use it everywere, so
func_extractor. extractor is not a good name either (and most
likely not a good division of responsibilities), but I'll get
to it later.

> +    functional_key.c

func_key

when a patch adds new modules the commit comment should contain
an explanation about what these modules do and why they were
added.


> index 1dbfe6b26..fdf38598e 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -30,6 +30,7 @@
>   */
>  #include "alter.h"
>  #include "assoc.h"
> +#include "box.h"

Kirill,

please stop bloating header file dependencies, especially circular
ones. If you need to include a dependency like this, you haven't
dealt with technical debt. There should be a separate patch which
improves modules structure so that this dependency is avoided.

The dependency  issue is described in the SOP, so I should not
have to point it out time and again.

-- 
Konstantin Osipov, Moscow, Russia