From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 14/14] WIP: module api: add box_key_def_validate_key() Date: Fri, 9 Oct 2020 04:22:53 +0300 [thread overview] Message-ID: <20201009012253.z43gxxrmtxfedksk@tkn_work_nb> (raw) In-Reply-To: <9ed17785-7743-827f-2fb3-d38627311699@tarantool.org> > > +int > > +box_key_def_validate_key(const box_key_def_t *key_def, const char *key, > > + bool allow_nullable) > > I don't like exposing such internal parameters as 'allow_nullable'. It > took me some time to find why is it even needed. Maybe better export 2 > separate functions? > > box_key_def_validate_exact_key > box_key_def_validate_range_key > > Or > > box_key_def_validate_unique_key > box_key_def_validate_key > > Or > > box_key_def_validate_unique_key > box_key_def_validate_non_unique_key > > Or > > box_key_def_validate_primary_key > box_key_def_validate_secondary_key > > Also did you think about making this function not setting any error? > What if a module code does not want to spoil diag, and just wants > to check the key? So as to set an own error later. Maybe would be > more flexible to expose a simple 'is_valid' instead of 'validate'? To be honest, I don't like 'allow_nullable' name too. I had guess that it would be more intuitive than 'is_exact', but it seems, it is not. In fact, the flag is like a hack to validate a key against a key_def as if all parts would be defined as non-nullable. Nothing prevents a user to create the same key_def, but non-nullable (_dump_parts(), change nullability of parts, call _new_v2()). So I will just remove the flag. If we'll really need it, we surely will able to add a function that will be named using words 'exact' and 'key'. Aside of this, please, look at the discussion around names of tuple and key_def related functions in [1]. It relates to this function as well. [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/019989.html > > Or do we do that to get a more precise error message and this is > more important than not setting a diag when not needed? I would say 'yes'. The diagnostic area is a kind of errno and I don't see anything bad in filling it even when not any caller will read the error.
next prev parent reply other threads:[~2020-10-09 1:22 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-23 1:14 [Tarantool-patches] [PATCH 00/14] RFC: module api: extend for external key_def Lua module Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 01/14] module api: get rid of typedef redefinitions Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 02/14] WIP: module api: expose box region Alexander Turenko 2020-09-24 22:31 ` Vladislav Shpilevoy 2020-10-08 19:21 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 03/14] WIP: module api/lua: add luaL_iscdata() function Alexander Turenko 2020-09-24 22:32 ` Vladislav Shpilevoy 2020-10-08 21:46 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 04/14] WIP: module api/lua: expose luaT_tuple_new() Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 05/14] WIP: module api/lua: add luaT_tuple_encode() Alexander Turenko 2020-09-24 22:32 ` Vladislav Shpilevoy 2020-10-12 19:06 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 06/14] WIP: refactoring: add API_EXPORT to lua/tuple functions Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 07/14] WIP: refactoring: add API_EXPORT to key_def functions Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 08/14] WIP: refactoring: extract key_def module API functions Alexander Turenko 2020-09-25 22:58 ` Vladislav Shpilevoy 2020-10-07 11:30 ` Alexander Turenko 2020-10-07 22:12 ` Vladislav Shpilevoy 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 09/14] WIP: module api: add box_key_def_new_ex() Alexander Turenko 2020-09-25 22:58 ` Vladislav Shpilevoy 2020-10-09 21:54 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 10/14] WIP: module api: add box_key_def_dump_parts() Alexander Turenko 2020-09-25 22:58 ` Vladislav Shpilevoy 2020-10-09 9:33 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 11/14] WIP: module api: expose box_tuple_validate_key_parts() Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 12/14] WIP: module api: expose box_key_def_merge() Alexander Turenko 2020-09-25 22:58 ` Vladislav Shpilevoy 2020-10-09 1:46 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 13/14] WIP: module api: expose box_tuple_extract_key_ex() Alexander Turenko 2020-09-25 22:58 ` Vladislav Shpilevoy 2020-10-09 1:14 ` Alexander Turenko 2020-10-10 1:21 ` Alexander Turenko 2020-09-23 1:14 ` [Tarantool-patches] [PATCH 14/14] WIP: module api: add box_key_def_validate_key() Alexander Turenko 2020-09-25 22:59 ` Vladislav Shpilevoy 2020-10-09 1:22 ` Alexander Turenko [this message] 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 00/16] RFC: module api: extend for external key_def Lua module Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 01/16] collation: allow to find a collation by a name Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 02/16] refactoring: adjust contract of luaT_tuple_new() Alexander Turenko 2020-09-28 21:26 ` Vladislav Shpilevoy 2020-10-05 11:58 ` Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 03/16] module api: get rid of typedef redefinitions Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 04/16] WIP: module api: expose box region Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 05/16] WIP: module api/lua: add luaL_iscdata() function Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 06/16] WIP: module api/lua: expose luaT_tuple_new() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 07/16] WIP: module api/lua: add luaT_tuple_encode() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 08/16] WIP: refactoring: add API_EXPORT to lua/tuple functions Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 09/16] WIP: refactoring: add API_EXPORT to key_def functions Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 10/16] WIP: refactoring: extract key_def module API functions Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 11/16] WIP: module api: add box_key_def_new_ex() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 12/16] WIP: module api: add box_key_def_dump_parts() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 13/16] WIP: module api: expose box_tuple_validate_key_parts() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 14/16] WIP: module api: expose box_key_def_merge() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 15/16] WIP: module api: expose box_tuple_extract_key_ex() Alexander Turenko 2020-09-23 1:40 ` [Tarantool-patches] [PATCH 1.10 16/16] WIP: module api: add box_key_def_validate_key() Alexander Turenko 2020-10-05 7:26 ` [Tarantool-patches] [PATCH 00/14] RFC: module api: extend for external key_def Lua module Alexander Turenko
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=20201009012253.z43gxxrmtxfedksk@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 14/14] WIP: module api: add box_key_def_validate_key()' \ /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