From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 5 Dec 2018 12:52:14 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH v5 2/9] lib: implement JSON tree class for json library Message-ID: <20181205095214.763qjnpkphv64kqm@esperanza> References: <02671a3d0a2236ecd6e12c0bc51b7f5e39272a2f.1543229303.git.kshcherbatov@tarantool.org> <20181129173816.kprfjhki5o7ytfbl@esperanza> <3c7bb503-561c-19b0-1197-f714b6f384d4@tarantool.org> <20181204175412.dayx2wplbxi5rrfz@esperanza> <38c62fa1-190d-181f-621b-8185847055f7@tarantool.org> <20181205090740.lyt6ikf7wmivavqb@esperanza> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181205090740.lyt6ikf7wmivavqb@esperanza> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org, Kostya Osipov List-ID: On Wed, Dec 05, 2018 at 12:07:40PM +0300, Vladimir Davydov wrote: > On Wed, Dec 05, 2018 at 11:37:06AM +0300, Kirill Shcherbatov wrote: > > Hi! Thank you for review. > > > > >>> BTW, json array start indexing from 0, not 1 AFAIK. Starting indexing > > >>> from 1 looks weird to me. > > > > > > You left this comment from my previous review unattended. > > > > In fact, it is not so; we use [token.num - 1] to retrieve field. > > Let's better describe it in comment: > > /** > > * Array of child records. Indexes in this array > > * match [token.num-1] index for JSON_TOKEN_NUM type > > * and are allocated sequentially for JSON_TOKEN_STR child > > * tokens. > > */ > > struct json_token **children; > > This is weird: AFAIU json_lexer may return token.num equal to 0. > What happens if we try to insert such a token into a tree? I think > we should insert a token at children[token.num], not [toekn.num-1]. Discussed verbally. Agreed that we should pass index_base to json_lexer_create, as we do, for example, in case of tuple_update. json_lexer_next will subtract index_base from token.num before returning the token. json_tree will use token.num as is when inserting it into the children array. Tuple-related routines will pass TUPLE_INDEX_BASE to json_lexer_create when parsing user input so that Lua 1-base indexes are converted to 0-base.