[tarantool-patches] [PATCH v3 6/7] Add merger for tuples streams (C part)
Alexander Turenko
alexander.turenko at tarantool.org
Thu Apr 25 16:32:27 MSK 2019
On Thu, Apr 25, 2019 at 02:43:43PM +0300, Konstantin Osipov wrote:
> * Alexander Turenko <alexander.turenko at 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.
merge source or merger's source. I found it neat to have one prefix
'merger_' for all related structures and functions.
But okay, I'll check it with Vladimir and if he agree will change those
names.
>
> > +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.
merger is a special kind of a source, the different thing.
>
> 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?
key_def is here for comparisons, yep, but format is for creating tuples
(with needed offsets). I'm like 'merger_' prefix, because it makes clear
that it is something that a merger need to perform its work.
The name is very common, yep. 'Comparison/sort context' would be more
specific, but not exact: it would arise a question: 'why a format is a
part of comparison context'?
I'll discuss this point with Vladimir too.
> 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.
So you'll need to construct a format in Lua somehow. Moreover, you'll
need to create *right* format in Lua to make comparisons fast. Looks as
a way more complex approach.
Maybe I didn't get your idea.
More information about the Tarantool-patches
mailing list