<HTML><BODY><div><div>Thank you for the review. Fixed the comments:</div><hr><div><div>The patch fixes a bug for the previous commit when statistics on box.execute<br>was collected twice. This happened because sql_prepare_and_execute called<br>sql_execute under the hood, so there's no need to do rmean_collect in both<br>of them.</div><div> </div><div>Follow-up #4756<br>---</div><div>Issue:</div><div><a href="https://github.com/tarantool/tarantool/issues/4756">https://github.com/tarantool/tarantool/issues/4756</a> </div><div>Branch:</div><div><a href="https://github.com/tarantool/tarantool/commit/a9c688b7312c8dc786ea14246eb380f9b5a148cf">https://github.com/tarantool/tarantool/commit/a9c688b7312c8dc786ea14246eb380f9b5a148cf</a> </div><div><br> src/box/execute.c | 1 -<br> test/sql/iproto.result | 10 +++++++++-<br> test/sql/iproto.test.lua | 3 ++-<br> 3 files changed, 11 insertions(+), 3 deletions(-)</div><div> </div><div>diff --git a/src/box/execute.c b/src/box/execute.c<br>index 3daa09205..24f8529ec 100644<br>--- a/src/box/execute.c<br>+++ b/src/box/execute.c<br>@@ -735,7 +735,6 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,<br> if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0)<br> return -1;<br> assert(stmt != NULL);<br>- rmean_collect(rmean_box, IPROTO_EXECUTE, 1);<br> enum sql_serialization_format format = sql_column_count(stmt) > 0 ?<br> DQL_EXECUTE : DML_EXECUTE;<br> port_sql_create(port, stmt, format, true);</div><div> </div><div>diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>index a391307d1..44ba499a0 100644<br>--- a/test/sql/iproto.result<br>+++ b/test/sql/iproto.result<br>@@ -808,6 +808,14 @@ s:execute({42})<br> rows:<br> - [42]<br> ...<br>+box.execute('SELECT 1;')<br>+---<br>+- metadata:<br>+ - name: '1'<br>+ type: integer<br>+ rows:<br>+ - [1]<br>+...<br> res, err = box.unprepare(s)<br> ---<br> ...<br>@@ -815,7 +823,7 @@ assert(box.stat().PREPARE.total == p + 1)<br> ---<br> - true<br> ...<br>-assert(box.stat().EXECUTE.total == e + 1)<br>+assert(box.stat().EXECUTE.total == e + 2)<br> ---<br> - true<br> ...</div><div> </div><div>diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>index 9eac91d2c..d884577c5 100644<br>--- a/test/sql/iproto.test.lua<br>+++ b/test/sql/iproto.test.lua<br>@@ -250,10 +250,11 @@ e = box.stat().EXECUTE.total<br> <br> s = box.prepare([[ SELECT ?; ]])<br> s:execute({42})<br>+box.execute('SELECT 1;')<br> res, err = box.unprepare(s)<br> <br> assert(box.stat().PREPARE.total == p + 1)<br>-assert(box.stat().EXECUTE.total == e + 1)<br>+assert(box.stat().EXECUTE.total == e + 2)<br> <br> -- Cleanup xlog<br> box.snapshot()<br>-- <br>2.24.0</div></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Среда, 4 марта 2020, 1:39 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15832751961907619004_BODY">Thanks for the patch!<br><br>See 2 comments below.<br><br>> Calling prepare and execute did not update corresponding request statistics<br>> in the box.stat table. This happened because methods that collect stats were<br>> never called where they should have been.<br><br>1. I don't think we can make this commit have the same commit message<br>as the original commit (even with an amendment below). And it should not<br>be 'Closes'. This is rather 'Follow-up'.<br><br>> There was a bug in previous commit resulting in collecting statistics on<br>> box.execute twice in some cases.<br>> <br>> Closes #4756<br>> ---<br>> Issue:<br>> <a href="https://github.com/tarantool/tarantool/issues/4756" target="_blank">https://github.com/tarantool/tarantool/issues/4756</a><br>> Branch:<br>> <a href="https://github.com/tarantool/tarantool/tree/eljashm/gh-4756-hotfix-box-stat-execute-prepare" target="_blank">https://github.com/tarantool/tarantool/tree/eljashm/gh-4756-hotfix-box-stat-execute-prepare</a> <br>> > diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>> index a391307d1..0e046577d 100644<br>> --- a/test/sql/iproto.result<br>> +++ b/test/sql/iproto.result<br>> @@ -800,22 +800,22 @@ e = box.stat().EXECUTE.total<br>> s = box.prepare([[ SELECT ?; ]])<br>> ---<br>> ...<br>> -s:execute({42})<br>> +res, err = box.unprepare(s)<br>> ---<br>> -- metadata:<br>> - - name: '?'<br>> - type: integer<br>> - rows:<br>> - - [42]<br>> ...<br>> -res, err = box.unprepare(s)<br>> +box.execute('create table test (id integer primary key, a integer)')<br>> ---<br>> +- row_count: 1<br>> +...<br>> +box.execute('DROP TABLE test')<br><br>2. Is it important to call DDL? Would 'box.execute('SELECT 1;')' be<br>enough?</div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Maria Khaydich</div></div></div><div> </div></div></BODY></HTML>