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

[Konstantin Osipov]

* Kirill Shcherbatov <kshcherbatov at tarantool.org> [18/10/29 20:25]:

>  
> +static int
> +lbox_index_resolve_path(struct lua_State *L)
> +{

No comment for the function. What does this function do and what
is it used for?

> +	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? 

> -                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

Please check with the docs team all the new messages this patch is
adding to the server.

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? 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov