From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs Date: Fri, 13 Sep 2019 02:44:01 +0300 [thread overview] Message-ID: <20190912234401.5mwym6khahljjcf7@tkn_work_nb> (raw) In-Reply-To: <cover.1568055477.git.v.shpilevoy@tarantool.org> On Mon, Sep 09, 2019 at 09:00:06PM +0200, Vladislav Shpilevoy wrote: > The patchset fixes several problems about JSON and msgpack serializers. > > 1) msgpackffi didn't use msgpack serializer options; > 2) tuple serializer either; > 3) update of a serializer option was not reflected in its Lua representation; > 4) during serialization too nested tables are silently cropped and there was no > way to prevent it. > > Also it was discovered, that msgpackffi does not care *all* options. Not only > about max_depth. I am not sure if it is worth fixing here (or at all) though. I would not do it here. However it worth to file a follow up issue: we should either fix or document this behaviour. It seems that it may be very unexpected that space:update / upsert does not follow, say, encode_invalid_numbers (if a user use this option and see that it works for other box functions). > > Changes in v2: > - Renames; > - Default value a flag for whether we need to raise an error on a too nested msgpack (was crop, now error). > > Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-4434-box-lua-msgpack-cfg > Issue: https://github.com/tarantool/tarantool/issues/4434 Thank you! The pathset looks mostly okay for me. I gave LGTM (with some minor comments) for the first three patches. I would discuss the option name in the fourth patch. WBR, Alexander Turenko.
next prev parent reply other threads:[~2019-09-12 23:44 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-10 20:24 [tarantool-patches] " Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 1/4] app: serializers update now is reflected in Lua Vladislav Shpilevoy 2019-09-12 23:22 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option Vladislav Shpilevoy 2019-09-12 23:24 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple Vladislav Shpilevoy 2019-09-12 23:27 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 4/4] app: allow to raise an error on too nested tables Vladislav Shpilevoy 2019-09-12 23:32 ` [tarantool-patches] " Alexander Turenko 2019-09-13 22:32 ` Vladislav Shpilevoy 2019-09-10 20:25 ` [tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy 2019-09-12 23:44 ` Alexander Turenko [this message] 2019-09-13 22:32 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190912234401.5mwym6khahljjcf7@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox