[tarantool-patches] [PATCH] sql: fix row count calculation for DELETE optimization

[Nikita Pettik <korablev@tarantool.org>]

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
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3816-account-changes-for-simple-delete
Issue: https://github.com/tarantool/tarantool/issues/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 d7df84874..c3418008c 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 116832f90..f9c42fdec 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 1ba8476fc..3ff14d53a 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 b6afe9184..ff8bf2afc 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 7577d2795..d4c86ac2b 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 c29e1d051..45a39d19a 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.
-- 
2.15.1