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

[n.pettik]

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. 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180717/36a67f17/attachment.html>