Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: increment rowcount of FK alteration
@ 2019-04-10 13:27 Nikita Pettik
  2019-04-10 13:39 ` [tarantool-patches] " Vladislav Shpilevoy
  2019-04-12 14:17 ` Kirill Yukhin
  0 siblings, 2 replies; 3+ messages in thread
From: Nikita Pettik @ 2019-04-10 13:27 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Before this patch SQL statement which involves FK constraints creation
or drop didn't increment rowcount:

box.execute("ALTER TABLE t ADD CONSTRAINT fk1 FOREIGN KEY (b) REFERENCES parent (a);")
---
- rowcount: 0
...

This patch fixes this misbehaviour: accidentally VDBE was forgotten to
enable counting changes during ALTER TABLE ADD/DROP constraint.

Closes #4130
---
Branch: https://github.com/tarantool/tarantool/issues/4130
Issue: https://github.com/tarantool/tarantool/tree/np/gh-4130-increment-row-count-on-fk-alteration

 src/box/sql/build.c            |  8 ++++++--
 test/sql/errinj.result         |  2 +-
 test/sql/foreign-keys.result   | 24 +++++++++++++++++++++++-
 test/sql/foreign-keys.test.lua |  5 +++++
 4 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index a06ba3e03..b1ed64a01 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1067,8 +1067,10 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context,
 			  constr_tuple_reg + 9);
 	sqlVdbeAddOp3(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, 0,
 			  constr_tuple_reg + 9);
-	if (parse_context->create_table_def.new_space == NULL)
+	if (parse_context->create_table_def.new_space == NULL) {
+		sqlVdbeCountChanges(vdbe);
 		sqlVdbeChangeP5(vdbe, OPFLAG_NCHANGE);
+	}
 	save_record(parse_context, BOX_FK_CONSTRAINT_ID, constr_tuple_reg, 2,
 		    vdbe->nOp - 1);
 	sqlReleaseTempRange(parse_context, constr_tuple_reg, 10);
@@ -1902,7 +1904,9 @@ sql_drop_foreign_key(struct Parse *parse_context)
 	 * ALTER TABLE DROP CONSTRAINT statement, since whole
 	 * DROP TABLE always returns 1 (one) as a row count.
 	 */
-	sqlVdbeChangeP5(sqlGetVdbe(parse_context), OPFLAG_NCHANGE);
+	struct Vdbe *v = sqlGetVdbe(parse_context);
+	sqlVdbeCountChanges(v);
+	sqlVdbeChangeP5(v, OPFLAG_NCHANGE);
 }
 
 /**
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index 6836d4afb..ee36a387b 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -362,7 +362,7 @@ box.snapshot()
 ...
 box.execute("ALTER TABLE t3 ADD CONSTRAINT fk1 FOREIGN KEY (b) REFERENCES t3;")
 ---
-- row_count: 0
+- row_count: 1
 ...
 box.execute("INSERT INTO t3 VALUES(1, 1, 3);")
 ---
diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result
index 2331c59f8..66eb94b77 100644
--- a/test/sql/foreign-keys.result
+++ b/test/sql/foreign-keys.result
@@ -350,7 +350,17 @@ box.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) MATCH F
 ...
 box.execute('ALTER TABLE tc ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES tp(id) MATCH PARTIAL ON DELETE CASCADE ON UPDATE SET NULL')
 ---
-- row_count: 0
+- row_count: 1
+...
+-- Test that ADD/DROP CONSTRAINT return correct row_count value.
+--
+box.execute('SELECT row_count();')
+---
+- metadata:
+  - name: row_count()
+    type: integer
+  rows:
+  - [1]
 ...
 box.space._fk_constraint:select{}
 ---
@@ -358,6 +368,18 @@ box.space._fk_constraint:select{}
   - ['FK_CONSTRAINT_1_TC', 518, 517, false, 'full', 'set_null', 'no_action', [1],
     [1]]
 ...
+box.execute('ALTER TABLE tc DROP CONSTRAINT fk1;')
+---
+- row_count: 1
+...
+box.execute('SELECT row_count();')
+---
+- metadata:
+  - name: row_count()
+    type: integer
+  rows:
+  - [1]
+...
 box.execute('DROP TABLE tc')
 ---
 - row_count: 1
diff --git a/test/sql/foreign-keys.test.lua b/test/sql/foreign-keys.test.lua
index b5d1f589d..4ac5999d7 100644
--- a/test/sql/foreign-keys.test.lua
+++ b/test/sql/foreign-keys.test.lua
@@ -156,7 +156,12 @@ box.space.PARENT:drop()
 box.execute('CREATE TABLE tp (id INT PRIMARY KEY, a INT UNIQUE)')
 box.execute('CREATE TABLE tc (id INT PRIMARY KEY, a INT REFERENCES tp(a) MATCH FULL ON DELETE SET NULL)')
 box.execute('ALTER TABLE tc ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES tp(id) MATCH PARTIAL ON DELETE CASCADE ON UPDATE SET NULL')
+-- Test that ADD/DROP CONSTRAINT return correct row_count value.
+--
+box.execute('SELECT row_count();')
 box.space._fk_constraint:select{}
+box.execute('ALTER TABLE tc DROP CONSTRAINT fk1;')
+box.execute('SELECT row_count();')
 box.execute('DROP TABLE tc')
 box.execute('DROP TABLE tp')
 
-- 
2.15.1

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-12 14:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-10 13:27 [tarantool-patches] [PATCH] sql: increment rowcount of FK alteration Nikita Pettik
2019-04-10 13:39 ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-12 14:17 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox