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

  • From: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Tue, 17 Jul 2018 04:22:28 +0300

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. 


Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018