From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CB105430409 for ; Mon, 24 Aug 2020 10:12:42 +0300 (MSK) From: Oleg Babin References: <20200821162607.68179-1-olegrok@tarantool.org> <8b318f99-f2f3-9671-96e6-0119781c39d1@tarantool.org> Message-ID: <1f559012-7827-7eff-7f65-8363e349a961@tarantool.org> Date: Mon, 24 Aug 2020 10:12:39 +0300 MIME-Version: 1.0 In-Reply-To: <8b318f99-f2f3-9671-96e6-0119781c39d1@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH] exports: allow to use json tools via FFI List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for your comments. See my answers below. On 22/08/2020 00:56, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > On 21.08.2020 18:26,olegrok@tarantool.org wrote: >> From: Oleg Babin >> >> This patch exports some json functions to be used via FFI. We solve >> following problem: currently we don't have any tools to inspect >> jsonpaths. E.g. user wants to ban or restrict an access to some >> tuple fields, it could be some system fields. Tarantool doesn't >> have hidden fields and to solve such problem we should fairly >> parse input jsonpaths. Before this patch user should write its own >> or some external tools. This patch allows to use functions from >> built-in json-lexer directly from Tarantool via FFI. > I understand the problem - you don't want to implement your own tool, > but you must realize, that exporting some internal structs and functions > via FFI is not safe. We won't care about "backward compatibility" of > internal functions. And JSON code is quite mutable, it may change in > future, if we will rework multikeys, for example. And that will make > your code unusable on newer versions. Even probably inside one version, > because we won't bump version number just for the sake of internal code > change. I understand it's OK in general for FFI. If I use some internal functions it's my responsibility to fix my wrappers/libraries to use it if something will change. > Even worse is that you took not only general stuff like json_lexer_next_token, > but also json_path_multikey_offset - this is heavily internal, why do you > need it? I don't really need it. I exported it just for consistency (to have full set of json_path_* functions). > I can't upvote this patch, sorry. I propose you to implement a JSON path > parser in Lua, or copy-paste the C code from json.h/c. and create your > own C module. The whole json.c is 666 lines, including the multikey > support and json trees, which occupy most of the space and you don't need > them. The parser is trivial, one day work to implement it from the scratch > with the tests. Please, don't use the internal structs and functions. > > You can't just export everything you need if by luck it happened we have it > somewhere internally implemented in C. It is not safe. > > The only thing you need to implement the same JSON path parser is utf8 module, > and you have it. Creation of some new module will add new external dependency that I don't want to introduce. I'm not sure that performance of Lua-implemented module will quite high as in case with FFI. But OK I understand your point.