[tarantool-patches] Re: [PATCH] sql: swap FK masks during altering of space

[Vladislav Shpilevoy]

Hi! Thanks for the patch! LGTM.

On 13/09/2019 21:08, Nikita Pettik wrote:
> It was forgotten to swap old and new mask (holding fields involved into
> foreign key relation) during space alteration (lists of object
> representing FK metadata are swapped successfully). Since mask is vital
> and depending on its value different byte-codes implementing SQL query
> can be produced, this mistake resulted in assertion fault in debug build
> and wrong constraint check in release build. Let's fix this bug and swap
> masks as well as foreign key lists.
> 
> Closes #4495
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-4495-swap-fk-mask
> Issue: https://github.com/tarantool/tarantool/issues/4495
> 
>  src/box/alter.cc               |  1 +
>  test/sql/foreign-keys.result   | 38 ++++++++++++++++++++++++++++++++++++++
>  test/sql/foreign-keys.test.lua | 15 +++++++++++++++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index c0780d6da..499df1ca2 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -619,6 +619,7 @@ space_swap_fk_constraints(struct space *new_space, struct space *old_space)
>  		   &old_space->child_fk_constraint);
>  	rlist_swap(&new_space->parent_fk_constraint,
>  		   &old_space->parent_fk_constraint);
> +	SWAP(new_space->fk_constraint_mask, old_space->fk_constraint_mask);
>  }
>  
>  /**
> diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result
> index b8ff8f145..29538e816 100644
> --- a/test/sql/foreign-keys.result
> +++ b/test/sql/foreign-keys.result
> @@ -457,5 +457,43 @@ t1:drop()
>  t2:drop()
>  ---
>  ...
> +-- gh-4495: space alter resulted in foreign key mask reset.
> +-- Which in turn led to wrong byte-code generation. Make sure
> +-- that alter of space doesn't affect result of query execution.
> +--
> +box.execute("CREATE TABLE t (id TEXT PRIMARY KEY, a INTEGER NOT NULL);")
> +---
> +- row_count: 1
> +...
> +box.execute("CREATE TABLE s (t_id TEXT PRIMARY KEY, a INTEGER NOT NULL, FOREIGN KEY(t_id) REFERENCES t(id) ON DELETE CASCADE);")
> +---
> +- row_count: 1
> +...
> +box.space.T:insert({'abc', 1})
> +---
> +- ['abc', 1]
> +...
> +box.space.S:insert({'abc', 1})
> +---
> +- ['abc', 1]
> +...
> +box.execute("CREATE INDEX i ON s (t_id);")
> +---
> +- row_count: 1
> +...
> +box.execute("DELETE FROM t WHERE id = 'abc';")
> +---
> +- row_count: 1
> +...
> +box.space.T:select()
> +---
> +- []
> +...
> +box.space.S:drop()
> +---
> +...
> +box.space.T:drop()
> +---
> +...
>  --- Clean-up SQL DD hash.
>  -test_run:cmd('restart server default with cleanup=1')
> diff --git a/test/sql/foreign-keys.test.lua b/test/sql/foreign-keys.test.lua
> index 4ac5999d7..d2dd88d28 100644
> --- a/test/sql/foreign-keys.test.lua
> +++ b/test/sql/foreign-keys.test.lua
> @@ -194,5 +194,20 @@ box.execute("ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY (id) REFERENCES t1;")
>  t1:drop()
>  t2:drop()
>  
> +-- gh-4495: space alter resulted in foreign key mask reset.
> +-- Which in turn led to wrong byte-code generation. Make sure
> +-- that alter of space doesn't affect result of query execution.
> +--
> +box.execute("CREATE TABLE t (id TEXT PRIMARY KEY, a INTEGER NOT NULL);")
> +box.execute("CREATE TABLE s (t_id TEXT PRIMARY KEY, a INTEGER NOT NULL, FOREIGN KEY(t_id) REFERENCES t(id) ON DELETE CASCADE);")
> +box.space.T:insert({'abc', 1})
> +box.space.S:insert({'abc', 1})
> +box.execute("CREATE INDEX i ON s (t_id);")
> +box.execute("DELETE FROM t WHERE id = 'abc';")
> +box.space.T:select()
> +
> +box.space.S:drop()
> +box.space.T:drop()
> +
>  --- Clean-up SQL DD hash.
>  -test_run:cmd('restart server default with cleanup=1')
>