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

Nikita Pettik korablev at tarantool.org
Mon Nov 19 15:12:56 MSK 2018


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





More information about the Tarantool-patches mailing list