From a693de5080b438780fb6a52dbf8dc1ed93dc4573 Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Mon, 19 Nov 2018 02:13:58 +0300
Subject: [PATCH] sql: fix row count calculation for DELETE optimization

When SQL DELETE statement comes in most primitive from without WHERE
clause and foreign key constraints, it is optimized and processed with
one VDBE instruction (instead of several OP_Delete). However, it was
forgotten to account affected tuples by row counter. Current patch fixes
this obvious defect.

Closes #3816
---
 src/box/sql.c               |  5 +++-
 src/box/sql/delete.c        |  1 +
 src/box/sql/tarantoolInt.h  |  2 +-
 src/box/sql/vdbe.c          | 10 +++++--
 test/sql/row-count.result   | 54 +++++++++++++++++++++++++++++++++++++
 test/sql/row-count.test.lua | 20 ++++++++++++++
 6 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index d7df848746..c3418008c6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -579,8 +579,10 @@ int tarantoolSqlite3EphemeralClearTable(BtCursor *pCur)
  * Removes all instances from table.
  * Iterate through the space and delete one by one all tuples.
  */
-int tarantoolSqlite3ClearTable(struct space *space)
+int tarantoolSqlite3ClearTable(struct space *space, uint32_t *tuple_count)
 {
+	assert(tuple_count != NULL);
+	*tuple_count = 0;
 	uint32_t key_size;
 	box_tuple_t *tuple;
 	int rc;
@@ -602,6 +604,7 @@ int tarantoolSqlite3ClearTable(struct space *space)
 			iterator_delete(iter);
 			return SQL_TARANTOOL_DELETE_FAIL;
 		}
+		(*tuple_count)++;
 	}
 	iterator_delete(iter);
 
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 116832f906..f9c42fdecb 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -210,6 +210,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		assert(!is_view);
 
 		sqlite3VdbeAddOp1(v, OP_Clear, space->def->id);
+		sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
 
 		/* Do not start Tarantool's transaction in case of
 		 * truncate optimization. This is workaround until
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 1ba8476fcc..3ff14d53ac 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -58,7 +58,7 @@ int
 sql_delete_by_key(struct space *space, uint32_t iid, char *key,
 		  uint32_t key_size);
 
-int tarantoolSqlite3ClearTable(struct space *space);
+int tarantoolSqlite3ClearTable(struct space *space, uint32_t *tuple_count);
 
 /**
  * Rename the table in _space. Update tuple with corresponding id
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b6afe9184e..ff8bf2afc2 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4644,7 +4644,7 @@ case OP_IdxGE:  {       /* jump */
 	break;
 }
 
-/* Opcode: Clear P1 P2 * * *
+/* Opcode: Clear P1 P2 * * P5
  * Synopsis: space id = P1
  * If P2 is not 0, use Truncate semantics.
  *
@@ -4652,6 +4652,9 @@ case OP_IdxGE:  {       /* jump */
  * in P1 argument. It is worth mentioning, that clearing routine
  * doesn't involve truncating, since it features completely
  * different mechanism under hood.
+ *
+ * If the OPFLAG_NCHANGE flag is set, then the row change count
+ * is incremented by the number of deleted tuples.
  */
 case OP_Clear: {
 	assert(pOp->p1 > 0);
@@ -4663,7 +4666,10 @@ case OP_Clear: {
 		if (box_truncate(space_id) != 0)
 			rc = SQL_TARANTOOL_ERROR;
 	} else {
-		rc = tarantoolSqlite3ClearTable(space);
+		uint32_t tuple_count;
+		rc = tarantoolSqlite3ClearTable(space, &tuple_count);
+		if (rc == 0 && (pOp->p5 & OPFLAG_NCHANGE) != 0)
+			p->nChange += tuple_count;
 	}
 	if (rc) goto abort_due_to_error;
 	break;
diff --git a/test/sql/row-count.result b/test/sql/row-count.result
index 7577d27954..d4c86ac2bb 100644
--- a/test/sql/row-count.result
+++ b/test/sql/row-count.result
@@ -100,6 +100,60 @@ box.sql.execute("SELECT ROW_COUNT();")
 ---
 - - [3]
 ...
+-- gh-3816: DELETE optimization returns valid number of
+-- deleted tuples.
+--
+box.sql.execute("DELETE FROM t3 WHERE 0 = 0;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [3]
+...
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+---
+...
+box.sql.execute("DELETE FROM t3")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [3]
+...
+-- But triggers still should't be accounted.
+--
+box.sql.execute("CREATE TABLE tt1 (id INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE TABLE tt2 (id INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE TRIGGER tr1 AFTER DELETE ON tt1 BEGIN DELETE FROM tt2; END;")
+---
+...
+box.sql.execute("INSERT INTO tt1 VALUES (1), (2), (3);")
+---
+...
+box.sql.execute("INSERT INTO tt2 VALUES (1), (2), (3);")
+---
+...
+box.sql.execute("DELETE FROM tt1 WHERE id = 2;")
+---
+...
+box.sql.execute("SELECT ROW_COUNT();")
+---
+- - [1]
+...
+box.sql.execute("SELECT * FROM tt2;")
+---
+- []
+...
+box.sql.execute("DROP TABLE tt1;")
+---
+...
+box.sql.execute("DROP TABLE tt2;")
+---
+...
 -- All statements which are not accounted as DML should
 -- return 0 (zero) as a row count.
 --
diff --git a/test/sql/row-count.test.lua b/test/sql/row-count.test.lua
index c29e1d051f..45a39d19ac 100644
--- a/test/sql/row-count.test.lua
+++ b/test/sql/row-count.test.lua
@@ -31,6 +31,26 @@ 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();")
+-- gh-3816: DELETE optimization returns valid number of
+-- deleted tuples.
+--
+box.sql.execute("DELETE FROM t3 WHERE 0 = 0;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("INSERT INTO t3 VALUES (1, 1), (2, 2), (3, 3);")
+box.sql.execute("DELETE FROM t3")
+box.sql.execute("SELECT ROW_COUNT();")
+-- But triggers still should't be accounted.
+--
+box.sql.execute("CREATE TABLE tt1 (id INT PRIMARY KEY);")
+box.sql.execute("CREATE TABLE tt2 (id INT PRIMARY KEY);")
+box.sql.execute("CREATE TRIGGER tr1 AFTER DELETE ON tt1 BEGIN DELETE FROM tt2; END;")
+box.sql.execute("INSERT INTO tt1 VALUES (1), (2), (3);")
+box.sql.execute("INSERT INTO tt2 VALUES (1), (2), (3);")
+box.sql.execute("DELETE FROM tt1 WHERE id = 2;")
+box.sql.execute("SELECT ROW_COUNT();")
+box.sql.execute("SELECT * FROM tt2;")
+box.sql.execute("DROP TABLE tt1;")
+box.sql.execute("DROP TABLE tt2;")
 
 -- All statements which are not accounted as DML should
 -- return 0 (zero) as a row count.
-- 
GitLab