[tarantool-patches] Re: [PATCH v5 3/6] sql: introduce tuple_fetcher class

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 31/05/2019 22:45, Konstantin Osipov wrote:
> * Kirill Shcherbatov <kshcherbatov@tarantool.org> [19/05/31 20:34]:
>> Hi! Thank you for review.
> 
> We have a yet enother -er class. It seems not to be a coincidence
> that all these -er classes are coming from the same contributor.
> 
> Kirill, please rethink the design, I kind of hit the limit of
> active classes. 
> 

I would agree with Kirill here.

1) Number of classes does not tell anything how good code is. If a
task can be solved with a new class better and simpler than with
modification of an existing one, then it is ok. Classes genocide
does not solve any problem.

2) I do not understand this dig on Kirill. He works on SQL,
introduces brand new features, which have never been presented
in Tarantool before, in any kind. You could with the same manner
dig on yourself, when you added MemtxSpace, MemtxEngine, MemtxTreeIndex,
MemtxHashIndex etc Memtx* + Vinyl* classes. Or when we were adding
new classes for vinyl internals like vy_run_iterator, vy_write_iterator,
vy_cache_iterator, vy_any_other_shit_iterator etc. Or when *you* asked
Nikita to add these numerous useless classes like "create_table_def",
"create_constraint_def", "alter_def" (see parse_def.h for the others),
which just store some values earlier passed as function parameters.

A class is ok, if it solves one atomic simple task, IMO. In this
concrete patch tuple_fetcher solves the task of not copying tuple
data into Vdbe cells, but still fetching fields.

3) The last point by order, but not by importance - the whole 'fetch'
theme was your idea. And I good one, to be honest. You saw a proposed
solution weeks ago. I explained it in details. Even if you do not
like it now for an unknown reason, it is still good and should be
implemented. It makes checks *copy-free*. We do not copy any single
byte from a tuple. We only keep offsets to its parts. And even more -
multiple checks share the same fetcher now, and if a first check
filled the offset table, the next checks will reuse it.

Sorry, if you feel that this email insults you, that was not on
purpose. Just my thoughts about your actions.