[tarantool-patches] [PATCH] sql: disallow to rename space if it is referenced by view

[tarantool-patches] [PATCH] sql: disallow to rename space if it is referenced by view

[Nikita Pettik]

Before this patch it was allowed to rename space which is referenced by
a view. In turn, view contains SELECT statement in a raw form (i.e. as a
string) and it is not modified during renaming routine. Hence, after
renaming space still has referencing counter > 0, but no usage of view
is allowed (since execution of SELECT results in "Space does not
exist"). To avoid such situations, lets ban renaming space if its view
reference counter > 0.

Note that RENAME is ANSI extension, so different DBs behave in this case
in different ways - some of them allow to rename tables referenced by a
view (PostgreSQL), others - don't (Oracle).

Closes #3746
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3746-view-rename
Issue: https://github.com/tarantool/tarantool/issues/3746

 src/box/alter.cc       | 6 ++++++
 test/sql/view.result   | 6 ++++++
 test/sql/view.test.lua | 3 +++
 3 files changed, 15 insertions(+)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 029da029e..f3b267401 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1823,6 +1823,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 				  space_name(old_space),
 				  "can not convert a space to "
 				  "a view and vice versa");
+		if (strcmp(def->name, old_space->def->name) != 0 &&
+		    old_space->def->view_ref_count > 0)
+			tnt_raise(ClientError, ER_ALTER_SPACE,
+				  space_name(old_space),
+				  "can not rename space which is referenced by "
+				  "view");
 		/*
 		 * Allow change of space properties, but do it
 		 * in WAL-error-safe mode.
diff --git a/test/sql/view.result b/test/sql/view.result
index b211bcb2e..8adbe568c 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -69,6 +69,12 @@ box.schema.create_space('view', {view = true})
 ---
 - error: Illegal parameters, unexpected option 'view'
 ...
+-- Space referenced by a view can't be renamed.
+box.sql.execute("ALTER TABLE t1 RENAME TO new_name;")
+---
+- error: 'Can''t modify space ''T1'': can not rename space which is referenced by
+    view'
+...
 -- View can be created via straight insertion into _space.
 sp = box.schema.create_space('test');
 ---
diff --git a/test/sql/view.test.lua b/test/sql/view.test.lua
index a6269a1bf..ba4d0bb2c 100644
--- a/test/sql/view.test.lua
+++ b/test/sql/view.test.lua
@@ -33,6 +33,9 @@ box.space._space:replace(v1);
 -- Views can't be created via space_create().
 box.schema.create_space('view', {view = true})
 
+-- Space referenced by a view can't be renamed.
+box.sql.execute("ALTER TABLE t1 RENAME TO new_name;")
+
 -- View can be created via straight insertion into _space.
 sp = box.schema.create_space('test');
 raw_sp = box.space._space:get(sp.id):totable();
-- 
2.15.1

[tarantool-patches] Re: [PATCH] sql: disallow to rename space if it is referenced by view

[Vladislav Shpilevoy]

Hi! Thanks for the patch! LGTM.

On 09/12/2018 18:57, Nikita Pettik wrote:
> Before this patch it was allowed to rename space which is referenced by
> a view. In turn, view contains SELECT statement in a raw form (i.e. as a
> string) and it is not modified during renaming routine. Hence, after
> renaming space still has referencing counter > 0, but no usage of view
> is allowed (since execution of SELECT results in "Space does not
> exist"). To avoid such situations, lets ban renaming space if its view
> reference counter > 0.
> 
> Note that RENAME is ANSI extension, so different DBs behave in this case
> in different ways - some of them allow to rename tables referenced by a
> view (PostgreSQL), others - don't (Oracle).
> 
> Closes #3746
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3746-view-rename
> Issue: https://github.com/tarantool/tarantool/issues/3746
> 

[tarantool-patches] Re: [PATCH] sql: disallow to rename space if it is referenced by view

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [18/12/09 20:53]:
> Before this patch it was allowed to rename space which is referenced by
> a view. In turn, view contains SELECT statement in a raw form (i.e. as a
> string) and it is not modified during renaming routine. Hence, after
> renaming space still has referencing counter > 0, but no usage of view
> is allowed (since execution of SELECT results in "Space does not
> exist"). To avoid such situations, lets ban renaming space if its view
> reference counter > 0.
> 
> Note that RENAME is ANSI extension, so different DBs behave in this case
> in different ways - some of them allow to rename tables referenced by a
> view (PostgreSQL), others - don't (Oracle).
> 
> Closes #3746
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3746-view-rename
> Issue: https://github.com/tarantool/tarantool/issues/3746
> 
>  src/box/alter.cc       | 6 ++++++
>  test/sql/view.result   | 6 ++++++
>  test/sql/view.test.lua | 3 +++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 029da029e..f3b267401 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1823,6 +1823,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
>  				  space_name(old_space),
>  				  "can not convert a space to "
>  				  "a view and vice versa");
> +		if (strcmp(def->name, old_space->def->name) != 0 &&
> +		    old_space->def->view_ref_count > 0)
> +			tnt_raise(ClientError, ER_ALTER_SPACE,
> +				  space_name(old_space),
> +				  "can not rename space which is referenced by "
> +				  "view");

Please specify which view is referencing this space in the error
message.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[tarantool-patches] Re: [PATCH] sql: disallow to rename space if it is referenced by view

[n.pettik]

[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]



> On 11 Dec 2018, at 10:02, Konstantin Osipov <kostja@tarantool.org> wrote:
> 
> * Nikita Pettik <korablev@tarantool.org <mailto:korablev@tarantool.org>> [18/12/09 20:53]:
>> Before this patch it was allowed to rename space which is referenced by
>> a view. In turn, view contains SELECT statement in a raw form (i.e. as a
>> string) and it is not modified during renaming routine. Hence, after
>> renaming space still has referencing counter > 0, but no usage of view
>> is allowed (since execution of SELECT results in "Space does not
>> exist"). To avoid such situations, lets ban renaming space if its view
>> reference counter > 0.
>> 
>> Note that RENAME is ANSI extension, so different DBs behave in this case
>> in different ways - some of them allow to rename tables referenced by a
>> view (PostgreSQL), others - don't (Oracle).
>> 
>> Closes #3746
>> ---
>> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3746-view-rename
>> Issue: https://github.com/tarantool/tarantool/issues/3746
>> 
>> src/box/alter.cc       | 6 ++++++
>> test/sql/view.result   | 6 ++++++
>> test/sql/view.test.lua | 3 +++
>> 3 files changed, 15 insertions(+)
>> 
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index 029da029e..f3b267401 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -1823,6 +1823,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
>> 				  space_name(old_space),
>> 				  "can not convert a space to "
>> 				  "a view and vice versa");
>> +		if (strcmp(def->name, old_space->def->name) != 0 &&
>> +		    old_space->def->view_ref_count > 0)
>> +			tnt_raise(ClientError, ER_ALTER_SPACE,
>> +				  space_name(old_space),
>> +				  "can not rename space which is referenced by "
>> +				  "view");
> 
> Please specify which view is referencing this space in the error
> message.

If I could do it without over-engineering, I would have already done it.
As you may see, space features only reference counter, so to get name
of view we should iterate over all VIEWs and compile SELECT for each VIEW.

> -- 
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 <tel:+7%20903%20626%2022%2032>
> http://tarantool.io <http://tarantool.io/> - www.twitter.com/kostja_osipov <http://www.twitter.com/kostja_osipov>

[-- Attachment #2: Type: text/html, Size: 12594 bytes --]

[tarantool-patches] Re: [PATCH] sql: disallow to rename space if it is referenced by view

[Kirill Yukhin]

Hello,

On 09 Dec 18:57, Nikita Pettik wrote:
> Before this patch it was allowed to rename space which is referenced by
> a view. In turn, view contains SELECT statement in a raw form (i.e. as a
> string) and it is not modified during renaming routine. Hence, after
> renaming space still has referencing counter > 0, but no usage of view
> is allowed (since execution of SELECT results in "Space does not
> exist"). To avoid such situations, lets ban renaming space if its view
> reference counter > 0.
> 
> Note that RENAME is ANSI extension, so different DBs behave in this case
> in different ways - some of them allow to rename tables referenced by a
> view (PostgreSQL), others - don't (Oracle).
> 
> Closes #3746
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3746-view-rename
> Issue: https://github.com/tarantool/tarantool/issues/3746

I've checked your patch into 2.1 branch.

--
Regards, Kirill Yukhin

[tarantool-patches] Re: [PATCH] sql: disallow to rename space if it is referenced by view

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [18/12/09 20:53]:
> +		if (strcmp(def->name, old_space->def->name) != 0 &&
> +		    old_space->def->view_ref_count > 0)
> +			tnt_raise(ClientError, ER_ALTER_SPACE,
> +				  space_name(old_space),
> +				  "can not rename space which is referenced by "
> +				  "view");

referenced by a view (missing article).

Please check your error message with the docs team if you're not
100% confident the grammar is correct.

Thanks!


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov