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

[Nikita Pettik <korablev@tarantool.org>]

changes() is legacy function which came from SQLite. However, its name
is not perfect since it's "number of rows affected", not"number of rows
changed". Moreover, to make it real row count, we are nullifying counter
representing it in case operation is not accounted for DML or DDL.
For example, before this patch behavior was like:

INSERT INTO t VALUES (1), (2), (3);
START TRANSACTION;
SELECT changes();
---
- - [3]
...

As one can see, row counter remained unchanged from last INSERT
operation. However, now START TRANSACTION set it to 0 and as a
consequence row_count() also returns 0.

Closes #2181
---
 src/box/execute.c           |   2 +-
 src/box/sql/func.c          |  17 +----
 src/box/sql/main.c          |  17 ++---
 src/box/sql/sqliteInt.h     |  11 +++-
 src/box/sql/vdbeaux.c       |  15 +++--
 test/sql/row-count.result   | 157 ++++++++++++++++++++++++++++++++++++++++++++
 test/sql/row-count.test.lua |  54 +++++++++++++++
 7 files changed, 234 insertions(+), 39 deletions(-)
 create mode 100644 test/sql/row-count.result
 create mode 100644 test/sql/row-count.test.lua

diff --git a/src/box/execute.c b/src/box/execute.c
index d75487498..e45044488 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -649,7 +649,7 @@ err:
 		if (iproto_reply_map_key(out, map_size, IPROTO_SQL_INFO) != 0)
 			goto err;
 		uint64_t id_count = 0;
-		int changes = sqlite3_changes(db);
+		int changes = db->nChange;
 		int size = mp_sizeof_uint(SQL_INFO_ROW_COUNT) +
 			   mp_sizeof_uint(changes);
 		if (!stailq_empty(autoinc_id_list)) {
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 5ab64821b..f0bf81ac3 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -585,21 +585,6 @@ randomBlob(sqlite3_context * context, int argc, sqlite3_value ** argv)
 	}
 }
 
-/*
- * Implementation of the changes() SQL function.
- *
- * IMP: R-62073-11209 The changes() SQL function is a wrapper
- * around the sqlite3_changes() C/C++ function and hence follows the same
- * rules for counting changes.
- */
-static void
-changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2)
-{
-	sqlite3 *db = sqlite3_context_db_handle(context);
-	UNUSED_PARAMETER2(NotUsed, NotUsed2);
-	sqlite3_result_int(context, sqlite3_changes(db));
-}
-
 /*
  * A structure defining how to do GLOB-style comparisons.
  */
@@ -1853,7 +1838,7 @@ sqlite3RegisterBuiltinFunctions(void)
 		FUNCTION(nullif, 2, 0, 1, nullifFunc, 0),
 		FUNCTION(version, 0, 0, 0, sql_func_version, AFFINITY_TEXT),
 		FUNCTION(quote, 1, 0, 0, quoteFunc, AFFINITY_TEXT),
-		VFUNCTION(changes, 0, 0, 0, changes, AFFINITY_INTEGER),
+		VFUNCTION(row_count, 0, 0, 0, sql_row_count, AFFINITY_INTEGER),
 		FUNCTION(replace, 3, 0, 0, replaceFunc, AFFINITY_TEXT),
 		FUNCTION(zeroblob, 1, 0, 0, zeroblobFunc, AFFINITY_BLOB),
 		FUNCTION(substr, 2, 0, 0, substrFunc, AFFINITY_TEXT),
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 76c152c68..8574d6464 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -483,19 +483,12 @@ setupLookaside(sqlite3 * db, void *pBuf, int sz, int cnt)
 	return SQLITE_OK;
 }
 
-/*
- * Return the number of changes in the most recent call to sqlite3_exec().
- */
-int
-sqlite3_changes(sqlite3 * db)
+void
+sql_row_count(struct sqlite3_context *context, MAYBE_UNUSED int unused1,
+	      MAYBE_UNUSED sqlite3_value **unused2)
 {
-#ifdef SQLITE_ENABLE_API_ARMOR
-	if (!sqlite3SafetyCheckOk(db)) {
-		(void)SQLITE_MISUSE_BKPT;
-		return 0;
-	}
-#endif
-	return db->nChange;
+	sqlite3 *db = sqlite3_context_db_handle(context);
+	sqlite3_result_int(context, db->nChange);
 }
 
 /*
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 73ca35c11..6247e12d7 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -654,8 +654,12 @@ sqlite3_user_data(sqlite3_context *);
 void
 sqlite3_randomness(int N, void *P);
 
-int
-sqlite3_changes(sqlite3 *);
+/**
+ * Return the number of affected rows in the last SQL statement.
+ */
+void
+sql_row_count(struct sqlite3_context *context, MAYBE_UNUSED int unused1,
+	      MAYBE_UNUSED sqlite3_value **unused2);
 
 void *
 sqlite3_user_data(sqlite3_context *);
@@ -1512,7 +1516,8 @@ struct sqlite3 {
 	u8 suppressErr;		/* Do not issue error messages if true */
 	u8 mTrace;		/* zero or more SQLITE_TRACE flags */
 	u32 magic;		/* Magic number for detect library misuse */
-	int nChange;		/* Value returned by sqlite3_changes() */
+	/** Value returned by sql_row_count(). */
+	int nChange;
 	int aLimit[SQLITE_N_LIMIT];	/* Limits */
 	int nMaxSorterMmap;	/* Maximum size of regions mapped by sorter */
 	struct sqlite3InitInfo {	/* Information used during initialization */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index c0945d36d..615a0f064 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2472,16 +2472,17 @@ sqlite3VdbeHalt(Vdbe * p)
 			}
 		}
 
-		/* If this was an INSERT, UPDATE or DELETE and no statement transaction
-		 * has been rolled back, update the database connection change-counter.
+		/*
+		 * If this was an INSERT, UPDATE or DELETE and
+		 * statement transaction has been rolled back,
+		 * update the database connection change-counter.
+		 * Other statements should return 0 (zero).
 		 */
 		if (p->changeCntOn) {
-			if (eStatementOp != SAVEPOINT_ROLLBACK) {
-				sqlite3VdbeSetChanges(db, p->nChange);
-			} else {
-				sqlite3VdbeSetChanges(db, 0);
-			}
+			sqlite3VdbeSetChanges(db, p->nChange);
 			p->nChange = 0;
+		} else {
+			db->nChange = 0;
 		}
 	}
 
diff --git a/test/sql/row-count.result b/test/sql/row-count.result
new file mode 100644
index 000000000..7577d2795
--- /dev/null
+++ b/test/sql/row-count.result
@@ -0,0 +1,157 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+-- Test cases concerning row count calculations.
+--
+box.sql.execute("CREATE TABLE t1 (s1 CHAR(10) PRIMARY KEY);")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("CREATE TABLE t2 (s1 CHAR(10) PRIMARY KEY, s2 CHAR(10) REFERENCES t1 ON DELETE CASCADE);")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("CREATE TABLE t3 (i1 INT PRIMARY KEY, i2 INT);")
+---
+...
+box.sql.execute("INSERT INTO t3 VALUES (0, 0);")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("CREATE TRIGGER x AFTER DELETE ON t1 FOR EACH ROW BEGIN UPDATE t3 SET i1 = i1 + ROW_COUNT(); END;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("INSERT INTO t1 VALUES ('a');")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("INSERT INTO t2 VALUES ('a','a');")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("INSERT INTO t1 VALUES ('b'), ('c'), ('d');")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [3]
+...
+-- REPLACE is accounted for two operations: DELETE + INSERT.
+box.sql.execute("REPLACE INTO t2 VALUES('a', 'c');")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [2]
+...
+box.sql.execute("DELETE FROM t1;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [4]
+...
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+---
+...
+box.sql.execute("TRUNCATE TABLE t3;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+---
+...
+box.sql.execute("UPDATE t3 SET i2 = 666;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [3]
+...
+-- All statements which are not accounted as DML should
+-- return 0 (zero) as a row count.
+--
+box.sql.execute("START TRANSACTION;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("COMMIT;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("COMMIT;")
+---
+- error: cannot commit - no transaction is active
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("ANALYZE;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute("EXPLAIN QUERY PLAN INSERT INTO t1 VALUES ('b'), ('c'), ('d');")
+---
+- - [0, 0, 0, 'SCAN TABLE T2']
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [0]
+...
+box.sql.execute('PRAGMA recursive_triggers')
+---
+- - [1]
+...
+-- Clean-up.
+--
+box.sql.execute("DROP TABLE t2;")
+---
+...
+box.sql.execute("DROP TABLE t3;")
+---
+...
+box.sql.execute("DROP TABLE t1;")
+---
+...
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
@@ -0,0 +1,54 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+-- Test cases concerning row count calculations.
+--
+box.sql.execute("CREATE TABLE t1 (s1 CHAR(10) PRIMARY KEY);")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("CREATE TABLE t2 (s1 CHAR(10) PRIMARY KEY, s2 CHAR(10) REFERENCES t1 ON DELETE CASCADE);")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("CREATE TABLE t3 (i1 INT PRIMARY KEY, i2 INT);")
+box.sql.execute("INSERT INTO t3 VALUES (0, 0);")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("CREATE TRIGGER x AFTER DELETE ON t1 FOR EACH ROW BEGIN UPDATE t3 SET i1 = i1 + ROW_COUNT(); END;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t1 VALUES ('a');")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t2 VALUES ('a','a');")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t1 VALUES ('b'), ('c'), ('d');")
+box.sql.execute("SELECT ROW_COUNT();")
+-- REPLACE is accounted for two operations: DELETE + INSERT.
+box.sql.execute("REPLACE INTO t2 VALUES('a', 'c');")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("DELETE FROM t1;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+box.sql.execute("TRUNCATE TABLE t3;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+box.sql.execute("UPDATE t3 SET i2 = 666;")
+box.sql.execute("SELECT ROW_COUNT();")
+
+-- All statements which are not accounted as DML should
+-- return 0 (zero) as a row count.
+--
+box.sql.execute("START TRANSACTION;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("COMMIT;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("COMMIT;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("ANALYZE;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("EXPLAIN QUERY PLAN INSERT INTO t1 VALUES ('b'), ('c'), ('d');")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute('PRAGMA recursive_triggers')
+
+-- 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.15.1