Re: [tarantool-patches] Re: [PATCH v5 12/12] box: specify indexes in user-friendly form

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> No comment for the function. What does this function do and what
> is it used for?
/**
 * Resolve field index by absolute JSON path first component and
 * return relative JSON path.
 */
> 
>> +	if (lua_gettop(L) != 3 ||
>> +	    !lua_isnumber(L, 1) || !lua_isnumber(L, 2) || !lua_isstring(L, 3)) {
>> +		return luaL_error(L, "Usage box.internal."
>> +				     "path_resolve(part_id, space_id, path)");
> 
> Why is it called path_resolve in one place and resolve_path in
> another? 
Ok, lbox_index_path_resolve
> 
>> -                end
>> -            end
>> -            if type(part.field) == 'string' then
>> +            local idx, path = box.internal.path_resolve(i, space_id, part.field)
>> +            if part.path ~= nil and part.path ~= path then
>>                  box.error(box.error.ILLEGAL_PARAMS,
>> -                          "options.parts[" .. i .. "]: field was not found by name '" .. part.field .. "'")
>> +                          "options.parts[" .. i .. "]: field path '"..
>> +                          part.path.." doesn't math path resolved by name '" ..
> 
> Doesn't match the path
Ok.

> 
> Please check with the docs team all the new messages this patch is
> adding to the server.
I'll ask Lena when the patch is ready. 

> I don't see how this cross-check help. I can change space format
> later on. Looks like we need to push the validation to alter.cc to
> ensure any kind of consistency.
> 
> Generally, as a rule, we try to avoid referencing anything by
> name, and prefer referencing by id, even though the user can use
> the name in box.* api. This spares us from the responsibility to 
> cross-check all the referencing objects whenever a referenced
> object changes.
> 
> What is the strategy for json paths here? Could you describe it in
> a comment? 
There are two ways to define JSON path:
1) manually specify fieldno and relative path -- this is cannonical approach introduced in
previous commits.
idx = s:create_index('test1', {parts = {{3, 'str', path = 'FIO.fname'}}})
2) use a fastpath introduced with this patch:
idx = s:create_index('test1', {parts = {{"user.FIO.fname", 'str'}}})

The second way is just user-friendly interface, string "user.FIO.fname" is not persisted in
Tarantool and indirectly converted to the (1) form in this patch.