[tarantool-patches] Re: [PATCH v4 06/20] refactoring: remove exceptions from schema_find_grants
Sergey Ostanevich
sergos at tarantool.org
Mon Oct 21 17:47:42 MSK 2019
LGTM.
On 21 Oct 15:29, Ilya Kosarev wrote:
>
> Hi!
>
> Thanks for your review.
> iterator_next is actually fine. It handles problems in a common way, as it is said in the comment in the header file:
> /**
> * Iterate to the next tuple.
> *
> * The tuple is returned in @ret (NULL if EOF).
> * Returns 0 on success, -1 on error.
> */
>
> It returns either what it->next(...) returns or 0 if we don't need to call it. This 0 return is not an error, although it is done under "invalidate" label.
> In case it->next(...) is called, the underlying function will be working as expected. diag_set & return -1 in case of error or return 0 if everything is fine.
>
> The same error handling is implemented in iterator_next_xc.
> Sincerely,
> Ilya Kosarev
>
>
> >Пятница, 18 октября 2019, 18:17 +03:00 от Sergey Ostanevich <sergos at tarantool.org>:
> >
> >Hi!
> >
> >LGTM with one nit - see below, follow up as a separate commit if needed.
> >
> >Sergos
> >
> >
> >On 23 Sep 18:56, Ilya Kosarev wrote:
> >> schema_find_grants is used in some triggers therefore it has to be
> >> cleared from exceptions. Now it doesn't throw any more.
> >> It's usages are updated.
> >>
> >> Part of #4247
> >> ---
> >> diff --git a/src/box/schema.cc b/src/box/schema.cc
> >> index 8d8aae448..9767207e0 100644
> >> --- a/src/box/schema.cc
> >> +++ b/src/box/schema.cc
> >> @@ -599,12 +599,22 @@ func_by_name(const char *name, uint32_t name_len)
> >> return (struct func *) mh_strnptr_node(funcs_by_name, func)->val;
> >> }
> >>
> >> -bool
> >> -schema_find_grants(const char *type, uint32_t id)
> >> +int
> >> +schema_find_grants(const char *type, uint32_t id, bool *out)
> >> {
> >> - struct space *priv = space_cache_find_xc(BOX_PRIV_ID);
> >> + struct space *priv = space_cache_find(BOX_PRIV_ID);
> >> + if (priv == NULL)
> >> + return -1;
> >> +
> >> /** "object" index */
> >> - struct index *index = index_find_system_xc(priv, 2);
> >> + if (!space_is_memtx(priv)) {
> >> + diag_set(ClientError, ER_UNSUPPORTED,
> >> + priv->engine->name, "system data");
> >> + return -1;
> >> + }
> >> + struct index *index = index_find(priv, 2);
> >> + if (index == NULL)
> >> + return -1;
> >> /*
> >> * +10 = max(mp_sizeof_uint32) +
> >> * max(mp_sizeof_strl(uint32)).
> >> @@ -612,9 +622,15 @@ schema_find_grants(const char *type, uint32_t id)
> >> char key[GRANT_NAME_MAX + 10];
> >> assert(strlen(type) <= GRANT_NAME_MAX);
> >> mp_encode_uint(mp_encode_str(key, type, strlen(type)), id);
> >> - struct iterator *it = index_create_iterator_xc(index, ITER_EQ, key, 2);
> >> + struct iterator *it = index_create_iterator(index, ITER_EQ, key, 2);
> >> + if (it == NULL)
> >> + return -1;
> >> IteratorGuard iter_guard(it);
> >> - return iterator_next_xc(it);
> >> + struct tuple *tuple;
> >> + if (iterator_next(it, &tuple) != 0)
> >> + return -1;
> >The iterator_next() has some unclear semantics with no diag set.
> >Please, reconsider the returned value and the diagnosis.
> >
> >> + *out = (tuple != NULL);
> >> + return 0;
> >> }
> >>
> >> struct sequence *
> >> diff --git a/src/box/schema.h b/src/box/schema.h
> >> index f9d15b38d..66555ab14 100644
> >> --- a/src/box/schema.h
> >> +++ b/src/box/schema.h
> >> @@ -185,11 +185,11 @@ func_cache_find(uint32_t fid)
> >> * Check whether or not an object has grants on it (restrict
> >> * constraint in drop object).
> >> * _priv space to look up by space id
> >> - * @retval true object has grants
> >> - * @retval false object has no grants
> >> + * @retval (bool *out) true object has grants
> >> + * @retval (bool *out) false object has no grants
> >> */
> >> -bool
> >> -schema_find_grants(const char *type, uint32_t id);
> >> +int
> >> +schema_find_grants(const char *type, uint32_t id, bool *out);
> >>
> >> /**
> >> * A wrapper around sequence_by_id() that raises an exception
> >> --
> >> 2.17.1
> >>
> >>
> >
>
>
> --
> Ilya Kosarev
> it->next
More information about the Tarantool-patches
mailing list