
<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Consider formatting: it is almost unreadable. Don’t add extra tabs to quote.</div><div class="">Also, I don’t see provided changes: you should attach them to your letter,</div><div class="">send in reply to this letter or send patch ver.2.</div><div class="">I can’t give you any review, just because there is no subject of it.</div><div class=""><br class=""></div><div class="">Please, rebase on fresh 2.0 and send diff or whole patch again.</div><div class="">Thanks.</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 4 Apr 2018, at 11:51, Bulat Niatshin <<a href="mailto:niatshin@tarantool.org" class="">niatshin@tarantool.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">

<div class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><br class=""></blockquote>Branch:<br class=""><a href="https://github.com/tarantool/tarantool/tree/bn/gh-3293-unique-opt" class="">https://github.com/tarantool/tarantool/tree/bn/gh-3293-unique-opt</a> <br class=""><br class="">Issue:<br class=""><a href="https://github.com/tarantool/tarantool/issues/3293" class="">https://github.com/tarantool/tarantool/issues/3293</a> <br class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class=""><br class="Apple-interchange-newline">In some cases ON CONFLICT REPLACE/IGNORE can be optimized. If<br class="">following conditions are true:<br class="">1) Space has PRIMARY KEY index only.<br class="">2) There are no foreign keys to other spaces.<br class="">3) There are no delete triggers to be fired if conflict happens.<br class="">4) The errt or action is REPLACE or IGNORE.</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">errt: typo.</span><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class=""> </span></div></blockquote><br class="">Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">Then uniqueness can be ensured by Tarantool only, without VDBE<br class="">bytecode. This patch contains a small fix for that +</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Replace ‘+’ with word. Don’t mention that it is ’small’ fix, let it be just fix.</span></div></blockquote><br class="">Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">related code was refactored a little bit and necessary comments</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">The same: don’t use ‘little bit’ — it sounds like you left smth</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">in a shitty state consciously.</span></div></blockquote><br class="">Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+/*<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* ABORT and DEFAULT error actions can be handled<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* by Tarantool only without emitting VDBE</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Remove word ‘only’: it confuses little bit (IMHO).</span></div></blockquote>Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+<br class="">+/*<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* Find out what action to take in case there is<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* a uniqueness conflict.<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>*/<br class="">+onError = pIdx->onError;<br class="">+</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Nit-picking: don’t put too many empty lines.</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">In this particular case, you outline very elementary statement.</span></div></blockquote><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class=""> </span></div></blockquote>Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+/*<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* override_error is an action which is directly<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* specified by user in queries like<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* INSERT OR <override_error> and therefore<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* should have higher priority than indexes<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* error actions.<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>*/</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">You put almost the same comment to on_error struct.</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Change it to more informative or remove.</span></div></blockquote><br class="">Done, this comment was removed.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+int override_error = on_conflict->override_error;<br class="">+int optimized_error_action = on_conflict->optimized_on_conflict;</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">If you used enum type, you wouldn’t have to add assertion below.</span></div></blockquote><br class="">Done, all error actions are enums right now.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+/*<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* override_error represents error action in queries like<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* INSERT/UPDATE OR REPLACE, INSERT/UPDATE OR IGNORE,<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* which is strictly specified by user and therefore<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* should have been used even when optimization is<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* possible (uniqueness can be checked by Tarantool).<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>*/</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">This comment is almost copy of one from struct declaration.<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span></span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Remove or rephrase.</span></div></blockquote><br class="">Done, removed.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+struct on_conflict {<br class="">+/**<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* Represents an error action in queries like<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* INSERT/UPDATE OR <override_error>, which<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* overrides all space constraints error actions.<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>*/<br class="">+int override_error;</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Why do you declare type of this field as int,</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">when it can take values from on_conflict_action enum?</span></div></blockquote><br class="">Done.<br class=""><br class=""><blockquote style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Helvetica, Arial, Tahoma, Verdana, sans-serif;" class=""><div class=""><blockquote style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" data-mce-style="font-family: Helvetica; font-size: 12px; text-size-adjust: auto;" class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* the value is ON_CONFLICT_ACTION_NONE, otherwise it is<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>* ON_CONFLICT_ACTON_IGNORE or ON_CONFLICT_ACTION_REPLACE.<br class="">+<span class="Apple-converted-space_mailru_css_attribute_postfix"> </span>*/<br class="">+int optimized_on_conflict;</blockquote><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Your structure is already called ‘on_conflict’,</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">so don’t repeat this phrase in field’s name.</span><br style="font-family: Helvetica; font-size: 12px;" data-mce-style="font-family: Helvetica; font-size: 12px;" class=""><span style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" data-mce-style="font-family: Helvetica; font-size: 12px; float: none; display: inline !important;" class="">Also, the same as previous field: better declare it as enum.</span></div></blockquote><br class="">Done.<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">-- <br class="">Bulat Niatshin</div>

</div></blockquote></div><br class=""></div></body></html>