From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Sergey Nikiforov <void@tarantool.org>, tarantool-patches@dev.tarantool.org Cc: Alexander Turenko <alexander.turenko@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v3] lua/key_def: fix compare_with_key() part count check Date: Mon, 21 Dec 2020 18:26:02 +0100 [thread overview] Message-ID: <c1952320-c3b5-6870-9da5-20578b5d2436@tarantool.org> (raw) In-Reply-To: <20201221122754.765559-1-void@tarantool.org> Hi! Thanks for the patch! It seems you didn't push the latest version on the branch. The diff below is different from what I see in `git show`. See 2 comments below. > diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c > index a781aeff9..5143ef3a4 100644 > --- a/src/box/lua/key_def.c > +++ b/src/box/lua/key_def.c > @@ -360,17 +360,14 @@ lbox_key_def_compare_with_key(struct lua_State *L) > struct region *region = &fiber()->gc; > size_t region_svp = region_used(region); > size_t key_len; > - const char *key_end, *key = lbox_encode_tuple_on_gc(L, 3, &key_len); > - uint32_t part_count = mp_decode_array(&key); > - if (key_validate_parts(key_def, key, part_count, true, > - &key_end) != 0) { > + const char *key = lbox_encode_tuple_on_gc(L, 3, &key_len); > + if (box_key_def_validate_key(key_def, key, NULL)) { > region_truncate(region, region_svp); > tuple_unref(tuple); > return luaT_error(L); > } > > - int rc = tuple_compare_with_key(tuple, HINT_NONE, key, > - part_count, HINT_NONE, key_def); > + int rc = box_tuple_compare_with_key(tuple, key, key_def); 1. Looks good. The old version with decoding the array only once also was fine to me though. Up to you and Alexander T. > region_truncate(region, region_svp); > tuple_unref(tuple); > lua_pushinteger(L, rc); > diff --git a/test/box-tap/gh-5307-key_def-part-count-check.test.lua b/test/box-tap/gh-5307-key_def-part-count-check.test.lua > new file mode 100755 > index 000000000..dfd50f799 > --- /dev/null > +++ b/test/box-tap/gh-5307-key_def-part-count-check.test.lua > @@ -0,0 +1,29 @@ > +#!/usr/bin/env tarantool > + > +local tap = require('tap') > +local mytest = tap.test('key_def part count tests') > + > +mytest:plan(3) > + > +local key_def = require('key_def') > +local kd = key_def.new({{fieldno = 1, type = 'unsigned'}}) > +local ok, res > + > +-- Should succeed > +ok, res = pcall(kd.compare_with_key, kd, {1}, {1}) > +print("First: ", ok, res) 2. Try to avoid print() function. By using print() you produce output not compatible with TAP, and complicate this issue: https://github.com/tarantool/tarantool/issues/5000 You can use test:diag() method to print any messages.
prev parent reply other threads:[~2020-12-21 17:26 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-21 12:27 Sergey Nikiforov 2020-12-21 17:26 ` Vladislav Shpilevoy [this message]
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=c1952320-c3b5-6870-9da5-20578b5d2436@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=void@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v3] lua/key_def: fix compare_with_key() part count check' \ /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