[tarantool-patches] Re: [PATCH v2] sql: add index_def to struct Index

["n.pettik" <korablev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

I have pushed some fixes and refactoring on branch.
Please, look at them and squash, if you are agree with them.
In case you disagree or have doubts - reply to this letter with
your comments. 

If they are OK to you, patch LGTM.
(Btw, you forgot to squash your last fixes with whole patch.)

>> I don’t understand: why do you need at all this cmp_def?
>> In SQL it is extremely unlikely to be useful.
> SQL has memtx/vinyl under it, where cmp_def is used, isn't it?
> As far as I understand, there is some mechanism, which creates
> struct index with struct index_def inside memtx (for example, for
> memtx_tree_create()) based on what we passed inside in
> createIndex() in sql/build.c
> Is it right? I am confused.
> If it is right, then we can simplify code in build by always 
> using key_def for cmp_def ?

These defs are stored only in our SQL hash. After tuple is inserted into _index,
the real key_def and cmp_def are created under the hood. And those
defs are used (see on_replace_dd_index()) in Tarantool's core.

So, I basically removed all ceremony with cmp_def during index building;
also I noticed that now only PK can feature ON REPLACE conflict action,
thus I simplified addIndexToTable routine. 



[-- Attachment #2: Type: text/html, Size: 8804 bytes --]