[tarantool-patches] Re: [PATCH v2 4/4] sql: rename changes() to row_count()

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

>> 1. As I understand, ROW_COUNT() should return > 0 only in a
>> not empty transaction. Here you got rowcount > 0 after DDL
>> transaction is committed. Also, twice in a row called
>> row_count() returning different values looks weird.
> 
>  From first sight - yes, but it makes sense:
> first call returns number of row count of preceding statement,
> which is 1 since table was created.
> second call again return number of row count of preceding statement,
> but now it is 0 since last statement was SELECT and it always
> return 0.
> 
>>
>> 'rowcount' should be returned as a metavalue from a DDL/DML
>> when box.sql.execute is removed.
> 
> I added this test on purpose so you can notice this behaviour.
> I’ve checked MariaDB and MySQL as Peter suggested and
> it works in this way (but row count is -1, not 0):
> 
> https://mariadb.com/kb/en/library/information-functions-row_count/
> 
> 	• For statements which return a result set (such as SELECT, SHOW, DESC or HELP),
>            returns -1, even when the result set is empty. This is also true for administrativ
>            statements, such as OPTIMIZE.
> 
> And ROW_COUNT() returns the number of updated row not only
> within active transaction. Again from docs:
> 
> "ROW_COUNT() returns the number of rows updated, inserted or deleted by the preceding statement. “
> 
> Only “preceding statement” is mentioned.
> 
> The same behaviour we can observe in MS Server:
> 
> https://docs.microsoft.com/ru-ru/sql/t-sql/functions/rowcount-transact-sql?view=sql-server-2017
> 
> “Every insert/update/select/set/delete statement resets the @@rowcount
>   to the rows affected by the executed statement.”
> 
> And the same it works in DB2:
> 
> https://www.ibm.com/support/knowledgecenter/en/SSEPEK_11.0.0/odbc/src/tpc/db2z_fnrowcount.html
> 
> If SQLRowCount() is executed after the SQLExecDirect() or SQLExecute()
> of an SQL statement other than INSERT, UPDATE, DELETE, or MERGE,
> it results in return code 0 and pcrow is set to -1.
> 
> So I guess current implementation now is quite close to others.

Thanks for the investigation! Then ok, decent.

The patchset LGTM.

> 
>>> diff --git a/test/sql/row-count.test.lua b/test/sql/row-count.test.lua
>>> new file mode 100644
>>> index 000000000..38d3520c2
>>> --- /dev/null
>>> +++ b/test/sql/row-count.test.lua
>>> +-- Clean-up.
>>> +--
>>> +box.sql.execute("DROP TABLE t2;")
>>> +box.sql.execute("DROP TABLE t3;")
>>> +box.sql.execute("DROP TABLE t1;")
>>> \ No newline at end of file
>>
>> 2. "No newline at end of file.”
> 
> Fixed.
>