[tarantool-patches] [PATCH] sql: increment rowcount of FK alteration

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

* [tarantool-patches] Re: [PATCH] sql: increment rowcount of FK alteration
  2019-04-10 13:27 [tarantool-patches] [PATCH] sql: increment rowcount of FK alteration Nikita Pettik
@ 2019-04-10 13:39 ` Vladislav Shpilevoy
  2019-04-12 14:17 ` Kirill Yukhin
  1 sibling, 0 replies; 3+ messages in thread
From: Vladislav Shpilevoy @ 2019-04-10 13:39 UTC (permalink / raw)
  To: Nikita Pettik, tarantool-patches, Kirill Yukhin

LGTM.

On 10/04/2019 16:27, Nikita Pettik wrote:
> 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

* [tarantool-patches] Re: [PATCH] sql: increment rowcount of FK alteration
  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
  1 sibling, 0 replies; 3+ messages in thread
From: Kirill Yukhin @ 2019-04-12 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Hello,

On 10 апр 16:27, Nikita Pettik wrote:
> 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

I've checked your patch into master & 2.1 branch.

--
Regards, Kirill Yukhin

^ 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