Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>,
	tarantool-patches@freelists.org,
	Kirill Yukhin <kyukhin@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] sql: increment rowcount of FK alteration
Date: Wed, 10 Apr 2019 16:39:17 +0300	[thread overview]
Message-ID: <c0bf4709-af0b-2d22-da8a-dffde8410663@tarantool.org> (raw)
In-Reply-To: <20190410132715.81284-1-korablev@tarantool.org>

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
> 

  reply	other threads:[~2019-04-10 13:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-10 13:27 [tarantool-patches] " Nikita Pettik
2019-04-10 13:39 ` Vladislav Shpilevoy [this message]
2019-04-12 14:17 ` [tarantool-patches] " Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0bf4709-af0b-2d22-da8a-dffde8410663@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: increment rowcount of FK alteration' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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