Re: [PATCH] sql: do not store key_def in VDBE op

[Kirill Yukhin <kyukhin@tarantool.org>]

Hello,
On 24 сен 20:49, Vladimir Davydov wrote:
> The parser must not deal with internal Tarantool objects, such as space,
> index, or key_def, directly, because it (a) violates encapsulation,
> turning the code into a convoluted mess, and (b) makes it impossible to
> run the parser and VDBE on different instances, which might come in
> handy for cluster SQL implementation. Instead, it should store plain
> names in the generated VDBE code. It may use objects the sole purpose
> of which is to represent object definitions, such as key_part_def or
> field_def, though.
> 
> This patch does a tiny step in this direction. It replaces key_def with
> sql_key_info in VDBE arguments. The new structure is a trivial wrapper
> around an array of key_part_def. It is ref-countable, just like its
> predecessor KeyInfo, so as to avoid unnecessary memory duplication.
> Since key_def spread roots deeply into the parser implementation, the
> new structure has two extra methods:
> 
>   sql_key_info_new_from_key_def
>   sql_key_info_to_key_def
> 
> so that it can be converted to/from a key definition. Note, the latter
> caches the result so as not to create a new key definition on subsequent
> function calls.
> 
> This partially undoes the work done by commit 501c6e28b095 ("sql:
> replace KeyInfo with key_def").
> 
> The reason why I'm doing this now is that I want to dispose of the
> key_def_set_part function, which is used extensively by the parser,
> because the latter stores key_def directly in VDBE op. This function
> has a vague semantic and rather obscures construction of a key
> definition. It will get especially nasty once JSON path indexes are
> introduced in Tarantool core. The new struct, sql_key_info, allows us
> to get rid of most of those calls.
> ---
> https://github.com/tarantool/tarantool/tree/merge-2.0
Your patch is OK for 2.0 branch.

--
Regards, Kirill Yukhin