Re: [tarantool-patches] [PATCH v3 6/7] Add merger for tuples streams (C part)

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Alexander Turenko <alexander.turenko@tarantool.org> [19/04/10 18:23]:
> +enum { MERGER_SOURCE_REF_MAX = INT_MAX };

It's merge_source, not merger, source, no? Here and in the rest of
the code.

> +void
> +merger_context_delete(struct merger_context *ctx)

Please decide it's either simply merger or merge_context.
merger_context does not make any sense to me.

what's in the context anyway? Can you have multiple contexts in a
single merge? If no, why not simply merger? Why do you need both,
a merger and a context?

> +/**
> + * Holds immutable parameters of a merger.
> + */
> +struct merger_context {
> +	struct key_def *key_def;
> +	box_tuple_format_t *format;
> +	/* Reference counter. */
> +	int refs;
> +};

Uhm.. Is it a comparison/sort context, not merger context then?
Please explain why you need to ref it btw. I would simply pass it
into Lua via ffi, to avoid any references, and make it clear that
the life cycle of this object is bound to the merge window.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov