Thank you for the review. Fixed the comments:

The patch fixes a bug for the previous commit when statistics on box.execute
was collected twice. This happened because sql_prepare_and_execute called
sql_execute under the hood, so there's no need to do rmean_collect in both
of them.

Follow-up #4756
---

Issue:

https://github.com/tarantool/tarantool/issues/4756

Branch:

https://github.com/tarantool/tarantool/commit/a9c688b7312c8dc786ea14246eb380f9b5a148cf

src/box/execute.c | 1 -
test/sql/iproto.result | 10 +++++++++-
test/sql/iproto.test.lua | 3 ++-
3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 3daa09205..24f8529ec 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -735,7 +735,6 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
    if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0)
        return -1;
    assert(stmt != NULL);
-   rmean_collect(rmean_box, IPROTO_EXECUTE, 1);
    enum sql_serialization_format format = sql_column_count(stmt) > 0 ?
                    DQL_EXECUTE : DML_EXECUTE;
    port_sql_create(port, stmt, format, true);

diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index a391307d1..44ba499a0 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -808,6 +808,14 @@ s:execute({42})
rows:
- [42]
...
+box.execute('SELECT 1;')
+---
+- metadata:
+ - name: '1'
+ type: integer
+ rows:
+ - [1]
+...
res, err = box.unprepare(s)
---
...
@@ -815,7 +823,7 @@ assert(box.stat().PREPARE.total == p + 1)
---
- true
...
-assert(box.stat().EXECUTE.total == e + 1)
+assert(box.stat().EXECUTE.total == e + 2)
---
- true
...

diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 9eac91d2c..d884577c5 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -250,10 +250,11 @@ e = box.stat().EXECUTE.total

s = box.prepare([[ SELECT ?; ]])
s:execute({42})
+box.execute('SELECT 1;')
res, err = box.unprepare(s)

assert(box.stat().PREPARE.total == p + 1)
-assert(box.stat().EXECUTE.total == e + 1)
+assert(box.stat().EXECUTE.total == e + 2)

-- Cleanup xlog
box.snapshot()
--
2.24.0

Среда, 4 марта 2020, 1:39 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:

Thanks for the patch!

See 2 comments below.

> Calling prepare and execute did not update corresponding request statistics
> in the box.stat table. This happened because methods that collect stats were
> never called where they should have been.

1. I don't think we can make this commit have the same commit message
as the original commit (even with an amendment below). And it should not
be 'Closes'. This is rather 'Follow-up'.

> There was a bug in previous commit resulting in collecting statistics on
> box.execute twice in some cases.
>
> Closes #4756
> ---
> Issue:
> https://github.com/tarantool/tarantool/issues/4756
> Branch:
> https://github.com/tarantool/tarantool/tree/eljashm/gh-4756-hotfix-box-stat-execute-prepare
> > diff --git a/test/sql/iproto.result b/test/sql/iproto.result
> index a391307d1..0e046577d 100644
> --- a/test/sql/iproto.result
> +++ b/test/sql/iproto.result
> @@ -800,22 +800,22 @@ e = box.stat().EXECUTE.total
> s = box.prepare([[ SELECT ?; ]])
> ---
> ...
> -s:execute({42})
> +res, err = box.unprepare(s)
> ---
> -- metadata:
> - - name: '?'
> - type: integer
> - rows:
> - - [42]
> ...
> -res, err = box.unprepare(s)
> +box.execute('create table test (id integer primary key, a integer)')
> ---
> +- row_count: 1
> +...
> +box.execute('DROP TABLE test')

2. Is it important to call DDL? Would 'box.execute('SELECT 1;')' be
enough?

--
Maria Khaydich