[Tarantool-patches] [PATCH] box: disallow to modify format of a view
Roman Khabibov
roman.habibov at tarantool.org
Fri Sep 25 13:18:48 MSK 2020
Hi! Thanks for the review.
> On Sep 24, 2020, at 23:59, Vladislav Shpilevoy <v.shpilevoy at tarantool.org> wrote:
>
> Hi! Thanks for the patch!
>
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index ba96d9c62..4d0da347a 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -2441,6 +2467,20 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
>> "view");
>> return -1;
>> }
>> + if (def->opts.is_view && old_space->def->field_count !=
>> + def->field_count) {
>> + diag_set(ClientError, ER_ALTER_SPACE,
>> + space_name(old_space),
>> + "can not modify field count of a view");
>> + return -1;
>> + }
>> + if (def->opts.is_view && format_is_changed(old_tuple,
>> + new_tuple)) {
>> + diag_set(ClientError, ER_ALTER_SPACE,
>> + space_name(old_space),
>> + "can not modify format of a view");
>> + return -1;
>> + }
>
> Wouldn't it be easier to just prohibit update of such
> tuple completely? Raise an error if new_tuple != NULL and
> old_tuple != NULL and the space is a view.
For some reason, I thought that you can rename views, because some
vendors allow it in ALTER TABLE RENAME TO. But I looked at our
standard file: you really can't rename a view. So that, you are right.
commit 515f2b8315fd266a1e9a13ab1b3357c680786cd5
Author: Roman Khabibov <roman.habibov at tarantool.org>
Date: Fri Sep 11 09:33:39 2020 +0300
box: disallow to alter view
Ban ability to modify view on box level. Since a view is a named
select, and not a space, in fact, altering view is not a valid
operation.
diff --git a/src/box/alter.cc b/src/box/alter.cc
index ba96d9c62..22578c9c4 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2398,6 +2398,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
}
} else { /* UPDATE, REPLACE */
assert(old_space != NULL && new_tuple != NULL);
+ if (old_space->def->opts.is_view) {
+ diag_set(ClientError, ER_ALTER_SPACE,
+ space_name(old_space),
+ "view may not be altered");
+ return -1;
+ }
struct space_def *def =
space_def_new_from_tuple(new_tuple, ER_ALTER_SPACE,
region);
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 14f6c1a97..54a8d5b80 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -60,11 +60,6 @@ sql_alter_table_rename(struct Parse *parse)
diag_set(ClientError, ER_NO_SUCH_SPACE, tbl_name);
goto tnt_error;
}
- if (space->def->opts.is_view) {
- diag_set(ClientError, ER_ALTER_SPACE, tbl_name,
- "view may not be altered");
- goto tnt_error;
- }
sql_set_multi_write(parse, false);
/* Drop and reload the internal table schema. */
struct Vdbe *v = sqlGetVdbe(parse);
diff --git a/test/sql/view.result b/test/sql/view.result
index 1e73ff621..b556b5482 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -44,7 +44,7 @@ v1[6]['view'] = false;
...
box.space._space:replace(v1);
---
-- error: 'Can''t modify space ''V1'': can not convert a space to a view and vice versa'
+- error: 'Can''t modify space ''V1'': view may not be altered'
...
t1 = box.space._space.index[2]:select('T1')[1]:totable();
---
@@ -68,7 +68,7 @@ v1[6]['view'] = true;
...
box.space._space:replace(v1);
---
-- error: Space declared as a view must have SQL statement
+- error: 'Can''t modify space ''V1'': view may not be altered'
...
-- Views can't be created via space_create().
box.schema.create_space('view', {view = true})
@@ -400,3 +400,106 @@ box.execute('DROP TABLE t1;')
---
- row_count: 1
...
+--
+-- Make sure that we can't alter a view.
+--
+box.execute("CREATE TABLE t1 (a INT PRIMARY KEY);")
+---
+- row_count: 1
+...
+box.execute("CREATE VIEW v AS SELECT * FROM t1;")
+---
+- row_count: 1
+...
+--
+-- Try to change owner.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+---
+...
+view[2] = 1
+---
+...
+box.space._space:replace(view)
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+--
+-- Try to rename.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+---
+...
+view[3] = 'a'
+---
+...
+box.space._space:replace(view)
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+--
+-- Try to change engine.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+---
+...
+view[4] = 'a'
+---
+...
+box.space._space:replace(view)
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+--
+-- Try to add a field.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+---
+...
+view_format = box.space.V:format()
+---
+...
+f = {type = 'string', nullable_action = 'none', name = 'B', is_nullable = true}
+---
+...
+table.insert(view_format, f)
+---
+...
+view[5] = 2
+---
+...
+view[7] = view_format
+---
+...
+box.space._space:replace(view)
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+--
+-- Try to modify format only.
+--
+view = box.space.V
+---
+...
+view:format{}
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+view_format = box.space.V:format()
+---
+...
+view_format[1].name = 'B'
+---
+...
+view:format(view_format)
+---
+- error: 'Can''t modify space ''V'': view may not be altered'
+...
+box.execute("DROP VIEW v;")
+---
+- row_count: 1
+...
+box.execute("DROP TABLE t1;")
+---
+- row_count: 1
+...
diff --git a/test/sql/view.test.lua b/test/sql/view.test.lua
index 47ca726af..2500c9e7a 100644
--- a/test/sql/view.test.lua
+++ b/test/sql/view.test.lua
@@ -138,3 +138,54 @@ box.execute('SELECT * FROM t1;')
box.execute('DROP VIEW v;')
box.execute('DROP TABLE t;')
box.execute('DROP TABLE t1;')
+
+--
+-- Make sure that we can't alter a view.
+--
+box.execute("CREATE TABLE t1 (a INT PRIMARY KEY);")
+box.execute("CREATE VIEW v AS SELECT * FROM t1;")
+
+--
+-- Try to change owner.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+view[2] = 1
+box.space._space:replace(view)
+
+--
+-- Try to rename.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+view[3] = 'a'
+box.space._space:replace(view)
+
+--
+-- Try to change engine.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+view[4] = 'a'
+box.space._space:replace(view)
+
+--
+-- Try to add a field.
+--
+view = box.space._space.index[2]:select('V')[1]:totable()
+view_format = box.space.V:format()
+f = {type = 'string', nullable_action = 'none', name = 'B', is_nullable = true}
+table.insert(view_format, f)
+view[5] = 2
+view[7] = view_format
+box.space._space:replace(view)
+
+--
+-- Try to modify format only.
+--
+view = box.space.V
+view:format{}
+
+view_format = box.space.V:format()
+view_format[1].name = 'B'
+view:format(view_format)
+
+box.execute("DROP VIEW v;")
+box.execute("DROP TABLE t1;")
More information about the Tarantool-patches
mailing list