<HTML><BODY><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review.<br></p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">iterator_next is actually fine. It handles problems in a common way, as it is said in the comment in the header file:<br></span></p><pre style="font-family: "DejaVu Sans Mono"; font-size: 9pt;"><span style="color:#808080;font-style:italic;">/**<br></span><span style="color:#808080;font-style:italic;"> * Iterate to the next tuple.<br></span><span style="color:#808080;font-style:italic;"> *<br></span><span style="color:#808080;font-style:italic;"> * The tuple is returned in @ret (NULL if EOF).<br></span><span style="color:#808080;font-style:italic;"> * Returns 0 on success, -1 on error.<br></span><span style="color:#808080;font-style:italic;"> */</span></pre><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><br>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.<br>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.<br><br></span>The same error handling is implemented in <span style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">iterator_next_xc.</span></p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;"><span style="color: #222222;font-family: Rubik, Arial, sans-serif;font-size: 17px;" data-mce-style="color: #222222; font-family: Rubik, Arial, sans-serif; font-size: 17px;">Sincerely,<br></span>Ilya Kosarev</p><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Пятница, 18 октября 2019, 18:17 +03:00 от Sergey Ostanevich <sergos@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15714118450834240661_BODY">Hi!<br>
<br>
LGTM with one nit - see below, follow up as a separate commit if needed.<br>
<br>
Sergos<br>
<br>
<br>
On 23 Sep 18:56, Ilya Kosarev wrote:<br>
> schema_find_grants is used in some triggers therefore it has to be<br>
> cleared from exceptions. Now it doesn't throw any more.<br>
> It's usages are updated.<br>
> <br>
> Part of #4247<br>
> ---<br>
> diff --git a/src/box/schema.cc b/src/box/schema.cc<br>
> index 8d8aae448..9767207e0 100644<br>
> --- a/src/box/schema.cc<br>
> +++ b/src/box/schema.cc<br>
> @@ -599,12 +599,22 @@ func_by_name(const char *name, uint32_t name_len)<br>
> return (struct func *) mh_strnptr_node(funcs_by_name, func)->val;<br>
> }<br>
> <br>
> -bool<br>
> -schema_find_grants(const char *type, uint32_t id)<br>
> +int<br>
> +schema_find_grants(const char *type, uint32_t id, bool *out)<br>
> {<br>
> - struct space *priv = space_cache_find_xc(BOX_PRIV_ID);<br>
> + struct space *priv = space_cache_find(BOX_PRIV_ID);<br>
> + if (priv == NULL)<br>
> + return -1;<br>
> +<br>
> /** "object" index */<br>
> - struct index *index = index_find_system_xc(priv, 2);<br>
> + if (!space_is_memtx(priv)) {<br>
> + diag_set(ClientError, ER_UNSUPPORTED,<br>
> + priv->engine->name, "system data");<br>
> + return -1;<br>
> + }<br>
> + struct index *index = index_find(priv, 2);<br>
> + if (index == NULL)<br>
> + return -1;<br>
> /*<br>
> * +10 = max(mp_sizeof_uint32) +<br>
> * max(mp_sizeof_strl(uint32)).<br>
> @@ -612,9 +622,15 @@ schema_find_grants(const char *type, uint32_t id)<br>
> char key[GRANT_NAME_MAX + 10];<br>
> assert(strlen(type) <= GRANT_NAME_MAX);<br>
> mp_encode_uint(mp_encode_str(key, type, strlen(type)), id);<br>
> - struct iterator *it = index_create_iterator_xc(index, ITER_EQ, key, 2);<br>
> + struct iterator *it = index_create_iterator(index, ITER_EQ, key, 2);<br>
> + if (it == NULL)<br>
> + return -1;<br>
> IteratorGuard iter_guard(it);<br>
> - return iterator_next_xc(it);<br>
> + struct tuple *tuple;<br>
> + if (iterator_next(it, &tuple) != 0)<br>
> + return -1;<br>
The iterator_next() has some unclear semantics with no diag set.<br>
Please, reconsider the returned value and the diagnosis.<br>
<br>
> + *out = (tuple != NULL);<br>
> + return 0;<br>
> }<br>
> <br>
> struct sequence *<br>
> diff --git a/src/box/schema.h b/src/box/schema.h<br>
> index f9d15b38d..66555ab14 100644<br>
> --- a/src/box/schema.h<br>
> +++ b/src/box/schema.h<br>
> @@ -185,11 +185,11 @@ func_cache_find(uint32_t fid)<br>
> * Check whether or not an object has grants on it (restrict<br>
> * constraint in drop object).<br>
> * _priv space to look up by space id<br>
> - * @retval true object has grants<br>
> - * @retval false object has no grants<br>
> + * @retval (bool *out) true object has grants<br>
> + * @retval (bool *out) false object has no grants<br>
> */<br>
> -bool<br>
> -schema_find_grants(const char *type, uint32_t id);<br>
> +int<br>
> +schema_find_grants(const char *type, uint32_t id, bool *out);<br>
> <br>
> /**<br>
> * A wrapper around sequence_by_id() that raises an exception<br>
> -- <br>
> 2.17.1<br>
> <br>
> <br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br><div id="_mcePaste" class="mcePaste" data-mce-bogus="1" style="position: absolute; left: 0px; top: -25px; width: 1px; height: 1px; overflow: hidden;"><pre style="font-family: "DejaVu Sans Mono"; font-size: 9pt;">it-><span style="color:#660e7a;">next</span></pre></div></BODY></HTML>