From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] sql: rework VIEW internals
Date: Wed, 6 Jun 2018 17:25:12 +0300 [thread overview]
Message-ID: <C32B3E1B-2CBD-4CB7-8505-34DEF2096FAC@tarantool.org> (raw)
In-Reply-To: <cf2ef4ad-5f21-c13c-edb0-1d66a79b2509@tarantool.org>
>> Firstly, I don't like code duplication in view on_commit/on_rollback
>> triggers. But I didn't manage to find any 'satisfactory' solution,
>> except for adding auxilary function with enum/bool arg which in turn
>> will determine action on view reference counter.
>
> I think it is better than code duplication. Lets do special function (maybe
> with another name)
>
> sql_view_update_references(Select *, int update_value)
> {
> expand_names()
> for (int i = 0; i < select->pSrc->nSrc; ++i) {
> const char *space_name = select->pSrc->a[i].zName;
> if (space_name == NULL)
> continue;
> uint32_t space_id = schema_find_id(BOX_SPACE_ID, 0, space_name,
> strlen(space_name));
> assert(space_id != BOX_ID_NIL);
> space = space_by_id(space_id);
> space->def->view_ref_count += update_value;
> }
> }
>
> on_drop/create_commit just do update_references(+1/-1) + delete.
Thx, Applied your diff with several minor additions:
/**
* Walk through all spaces from 'FROM' clause of given select,
* and update their view references counters.
*
* @param select Select to be
* @param update_value +1 on view creation, -1 on drop.
*/
static void
update_view_references(struct Select *select, int update_value)
{
assert(update_value == 1 || update_value == -1);
sql_select_expand_from_tables(select);
int from_tables_count = sql_select_from_tables_count(select);
for (int i = 0; i < from_tables_count; ++i) {
const char *space_name = sql_select_from_table_name(select, i);
if (space_name == NULL)
continue;
uint32_t space_id = schema_find_id(BOX_SPACE_ID, 2, space_name,
strlen(space_name));
assert(space_id != BOX_ID_NIL);
struct space *space = space_by_id(space_id);
assert(space->def->view_ref_count > 0 || update_value > 0);
space->def->view_ref_count += update_value;
}
}
>
>> Secondly, (I guess) including in alter.cc sqliteInt.h header seems to be
>> total mess, but again - I failed to come up with kind of sinsible> resolution. Is it time for dividing SQL code base into source/header files?
>
> Yes, sqliteInt.h in alter.cc looks ugly. Just implement temporary
> getters/setters taking Select * and defined in sql.h. Look for example on
> Expr * - it is used in alter.cc, but does not require include anything
> except sql.h.
>
> Also maybe the suggestion above can help you. You can declare
> sql_view_update_references in sql.h, and do not access
> select->pSrc->a[i].zName directly in alter.cc.
I can’t do this since schema_find_id() depends on cpp code.
>
>> At least, including "select.h" looks less annoying.
>> Finally, I temporary changed system space in box_space_id_by_name()
>> from _vspace to _space, in order to find space by name in on_dd_replace trigger.
>
> _vspace is needed in box_space_id_by_name because it is called by external API
> and hides from the user the spaces he has no access. So _space is not acceptable
> here.
>
>> I have noticed that Kirill Sh. has almost done the same thing, so this
>> hack is going to be substituted with his patch.
>
> In alter.cc you can use schema_find_id(). Kirill in his patch was not able to use
> this function because it throws exceptions, but alter.cc is C++.
>
> See my 22 comments and recommendations below.
>
> 1. Travis is failed on the branch on box/func_reload. It is very strange, but
> on the previous commit on your branch it is not failed.
Wow, it was really strange. Last version of patch seems to pass this test.
>
>> src/box/alter.cc | 84 ++++++++++++
>> src/box/box.cc | 4 +-
>> src/box/space_def.c | 1 +
>> src/box/space_def.h | 2 +
>> src/box/sql.h | 47 +++++++
>> src/box/sql/build.c | 289 ++++++++++++-----------------------------
>> src/box/sql/delete.c | 6 +-
>> src/box/sql/expr.c | 5 +-
>> src/box/sql/fkey.c | 4 +-
>> src/box/sql/insert.c | 4 +-
>> src/box/sql/parse.y | 19 +--
>> src/box/sql/select.c | 61 +++++++--
>> src/box/sql/sqliteInt.h | 31 ++++-
>> src/box/sql/tokenize.c | 18 +++
>> src/box/sql/trigger.c | 8 +-
>> src/box/sql/vdbe.c | 24 ++++
>> test/sql-tap/colname.test.lua | 4 +-
>> test/sql-tap/drop_all.test.lua | 4 +-
>> test/sql-tap/fkey2.test.lua | 1 +
>> test/sql-tap/trigger1.test.lua | 1 +
>> test/sql-tap/triggerC.test.lua | 1 +
>> test/sql-tap/view.test.lua | 137 ++++++++++++++-----
>> test/sql/view.result | 23 +++-
>> test/sql/view.test.lua | 11 +-
>> 24 files changed, 502 insertions(+), 287 deletions(-)
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index b62f8adea..1fbb54d1a 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -29,6 +29,7 @@
>> * SUCH DAMAGE.
>> */
>> #include "alter.h"
>> +#include "box.h"
>
> 2. As I said above, you can remove box.h. It must not be included
> anywhere in internal files when possible. So consider this diff,
> please.
>
> After this fix and after others I have not run tests. So please
> check them before applying. If they fail after me, then sorry %)
>
> @@ -29,7 +29,6 @@
> * SUCH DAMAGE.
> */
> #include "alter.h"
> -#include "box.h"
> #include "schema.h"
> @@ -1465,8 +1464,8 @@ on_create_view_commit(struct trigger *trigger, void *event)
> const char *space_name = select->pSrc->a[i].zName;
> if (space_name == NULL)
> continue;
> - uint32_t space_id = box_space_id_by_name(space_name,
> - strlen(space_name));
> + uint32_t space_id = schema_find_id(BOX_SPACE_ID, 2, space_name,
> + strlen(space_name));
> assert(space_id != BOX_ID_NIL);
> @@ -1497,8 +1496,8 @@ on_drop_view_commit(struct trigger *trigger, void *event)
> const char *space_name = select->pSrc->a[i].zName;
> if (space_name == NULL)
> continue;
> - uint32_t space_id = box_space_id_by_name(space_name,
> - strlen(space_name));
> + uint32_t space_id = schema_find_id(BOX_SPACE_ID, 2, space_name,
> + strlen(space_name));
> assert(space_id != BOX_ID_NIL);
>
Ok, got it. I applied your diff.
>
>> @@ -1440,6 +1442,70 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin,
>> }
>> }
>> +/**
>> + * Trigger which is fired on creation of new SQL view.
>> + * Its purpose is to increment view reference counters of
>> + * dependent spaces.
>> + */
>> +static void
>> +on_create_view_commit(struct trigger *trigger, void *event)
>> +{
>> + (void) event;
>> + struct space *space = (struct space *)trigger->data;
>> + struct space_def *def = space->def;
>> + struct Select *select;
>> + if (sql_view_compile(sql_get(), def->opts.sql, &select) != 0) {
>> + diag_log();> + return;
>
> 3. On commit trigger must never fail. So please, do compile in
> on_replace_dd_space, and here just use it. I propose this.
>
> - Compile sql in on_replace_dd_space;
> - Put compiled Select * in trigger->data (as well as you pass space * now);
> - In on_drop/create_view_commit you take Select * from trigger data. Space *
> as I can see is used in these functions to take def->sql and create
> Select * only so it is ok to replace it with already compiled Select *.
>
> Then do not forget to add on_rollback triggers to delete Select *.
Done:
+/**
+ * Trigger which is fired on creation of new SQL view.
+ * Its purpose is to increment view reference counters of
+ * dependent spaces.
+ */
+static void
+on_create_view_commit(struct trigger *trigger, void *event)
+{
+ (void) event;
+ struct Select *select = (struct Select *)trigger->data;
+ update_view_references(select, 1);
sql_select_delete(sql_get(), select);
}
@@ -1483,26 +1483,19 @@ static void
on_drop_view_commit(struct trigger *trigger, void *event)
{
(void) event;
- struct space *space = (struct space *)trigger->data;
- struct space_def *def = space->def;
- struct Select *select;
- if (sql_view_compile(sql_get(), def->opts.sql, &select) != 0) {
- diag_log();
- return;
- }
- if (select == NULL)
- return;
- sql_select_expand_from_tables(select);
- for (int i = 0; i < select->pSrc->nSrc; ++i) {
- const char *space_name = select->pSrc->a[i].zName;
- if (space_name == NULL)
- continue;
- uint32_t space_id = box_space_id_by_name(space_name,
- strlen(space_name));
- assert(space_id != BOX_ID_NIL);
- struct space *space = space_by_id(space_id);
- space->def->view_ref_count--;
- }
+ struct Select *select = (struct Select *)trigger->data;
+ update_view_references(select, -1);
+ sql_select_delete(sql_get(), select);
+}
+/**
+ * This trigger is invoked on drop/create of SQL view.
+ * Release memory for struct SELECT compiled in
+ * on_replace_dd_space trigger.
+ */
+static void
+on_alter_view_rollback(struct trigger *trigger, void *event)
+{
+ (void) event;
+ struct Select *select = (struct Select *)trigger->data;
sql_select_delete(sql_get(), select);
}
@@ -1611,11 +1606,19 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_create_space_commit, space);
txn_on_commit(txn, on_commit);
- if (def->opts.is_view && def->opts.sql != NULL) {
+ if (def->opts.is_view) {
+ struct Select *select = sql_view_compile(sql_get(),
+ def->opts.sql);
+ if (select == NULL)
+ diag_raise();
struct trigger *on_commit_view =
txn_alter_trigger_new(on_create_view_commit,
- space);
+ select);
txn_on_commit(txn, on_commit_view);
+ struct trigger *on_rollback_view =
+ txn_alter_trigger_new(on_alter_view_rollback,
+ select);
+ txn_on_rollback(txn, on_rollback_view);
}
@@ -1652,12 +1655,20 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_drop_space_commit, space);
txn_on_commit(txn, on_commit);
- if (old_space->def->opts.is_view &&
- old_space->def->opts.sql != NULL) {
+ if (old_space->def->opts.is_view) {
+ struct Select *select =
+ sql_view_compile(sql_get(),
+ old_space->def->opts.sql);
+ if (select == NULL)
+ diag_raise();
struct trigger *on_commit_view =
txn_alter_trigger_new(on_drop_view_commit,
- space);
+ select);
txn_on_commit(txn, on_commit_view);
+ struct trigger *on_rollback_view =
+ txn_alter_trigger_new(on_alter_view_rollback,
+ select);
+ txn_on_rollback(txn, on_rollback_view);
}
>
>> @@ -1575,6 +1652,13 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
>> struct trigger *on_commit =
>> txn_alter_trigger_new(on_drop_space_commit, space);
>> txn_on_commit(txn, on_commit);
>> + if (old_space->def->opts.is_view &&
>> + old_space->def->opts.sql != NULL) {
>
> 4. How is it possible that is_view == true, but sql == NULL?
No way. I have mistaken:
@@ -1611,7 +1605,12 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_create_space_commit, space);
txn_on_commit(txn, on_commit);
- if (def->opts.is_view && def->opts.sql != NULL) {
+ if (def->opts.is_view) {
@@ -1652,8 +1651,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_drop_space_commit, space);
txn_on_commit(txn, on_commit);
- if (old_space->def->opts.is_view &&
- old_space->def->opts.sql != NULL) {
+ if (old_space->def->opts.is_view) {
>
>> if (def->opts.is_view != old_space->def->opts.is_view)
>> tnt_raise(ClientError, ER_ALTER_SPACE,
>> space_name(old_space),
>> "can not convert a space to "
>> "a view and vice versa");
>
> 5. Please, use {} for 'if' body consisting of multiple lines.
This is not diff from my patch. Check on ref counts is surrounded by brackets.
>
> 6. I do not see where do you set def->view_ref_count on space alter.
> See this test:
>
> tarantool> box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY);")
> tarantool> box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1")
> tarantool> box.sql.execute("DROP TABLE t1")
> ---
> - error: 'Can''t drop table T1: other views depend on this space'
> ...
>
> It is ok. But then I do this:
>
> tarantool> s = box.space._space:get{box.space.T1.id}
> tarantool> box.space._space:replace(s)
>
> Nothing is really changed, only space * and space_def * are
> recreated. But now I am able to do this:
>
> tarantool> box.sql.execute("DROP TABLE t1")
> ---
> ...
>
> You need to update class ModifySpace to move view refs from the
> old to the new def.
@@ -946,6 +945,7 @@ ModifySpace::alter_def(struct alter_space *alter)
new_dict = new_def->dict;
new_def->dict = alter->old_space->def->dict;
tuple_dictionary_ref(new_def->dict);
+ new_def->view_ref_count = alter->old_space->def->view_ref_count;
+++ b/test/sql/view.test.lua
@@ -47,6 +47,14 @@ box.space.T2:drop();
box.sql.execute('DROP VIEW v2;');
box.sql.execute('DROP TABLE t2;');
+-- Check that alter transfers reference counter.
+box.sql.execute("CREATE TABLE t2(id INTEGER PRIMARY KEY);");
+box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;");
+box.sql.execute("DROP TABLE t2;");
+sp = box.space._space:get{box.space.T2.id};
+box.space._space:replace(sp);
+box.sql.execute("DROP TABLE t2”);
+box.sql.execute("DROP VIEW v2;");
+box.sql.execute("DROP TABLE t2;");
+
>
>> diff --git a/src/box/sql.h b/src/box/sql.h
>> index 23021e56b..9a006e5cb 100644
>> --- a/src/box/sql.h
>> +++ b/src/box/sql.h
>> @@ -83,6 +83,19 @@ int
>> sql_expr_compile(struct sqlite3 *db, const char *expr, int expr_len,
>> struct Expr **result);
>> +/**
>> + * This routine executes parser on 'CREATE VIEW ...' statement
>> + * and loads content of SELECT into internal structs as result.
>> + *
>> + * @param db Current SQL context.
>> + * @param view_stmt String containing 'CREATE VIEW' statement.
>> + * @param[out] select Fetched SELECT statement.
>> + * @retval 0 on success, -1 otherwise.
>> + */
>> +int
>> +sql_view_compile(struct sqlite3 *db, const char *view_stmt,
>> + struct Select **select);
>
> 7. You need an out parameter only when returned value is occupied for
> another output. Here it is enough to just return Select * on success
> and NULL on error. I see that sql_expr_compile uses out parameter but
> it is wrong too. Kirill in his patch fixes it.
Ok, I just copy-pasted code of compiling exprs.
*if-you-copy-paste-code-youre-gonna-have-a-bad-time.jpg*
+struct Select *
+sql_view_compile(struct sqlite3 *db, const char *view_stmt)
- *select = parser.parsed_select;
+ struct Select *result = parser.parsed_ast.select;
sql_parser_destroy(&parser);
- return 0;
+ return result;
>> @@ -293,6 +306,40 @@ sql_parser_create(struct Parse *parser, struct sqlite3 *db);
>> +/**
>> + * Work the same as sqlite3SrcListAppend(), but before adding to
>> + * list provide check on name duplicates: only values with unique
>> + * names are appended.
>> + *
>> + * @param db Database handler.
>> + * @param list List of entries.
>> + * @param new_name Name of entity to be added.
>> + * @retval @list with new element on success, old one otherwise.
>> + */
>> +struct SrcList *
>> +sql_src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
>> + const char *new_name);
>
> 8. Do you really need this declaration in sql.h? It is used in select.c
> only. Maybe it can be static inside this file?
That is my bad, I have forgotten to made it static. Fixed:
+++ b/src/box/sql.h
@@ -316,20 +316,6 @@ sql_parser_destroy(struct Parse *parser);
void
sql_select_delete(struct sqlite3 *db, struct Select *select);
-/**
- * Work the same as sqlite3SrcListAppend(), but before adding to
- * list provide check on name duplicates: only values with unique
- * names are appended.
- *
- * @param db Database handler.
- * @param list List of entries.
- * @param new_name Name of entity to be added.
- * @retval @list with new element on success, old one otherwise.
- */
-struct SrcList *
-sql_src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
- const char *new_name);
-
+++ b/src/box/sql/build.c
@@ -3498,22 +3498,6 @@ sqlite3SrcListAppend(sqlite3 * db, /* Connection to notify of malloc failures */
return pList;
}
-struct SrcList *
-sql_src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
- const char *new_name)
-{
- assert(list != NULL);
- assert(new_name != NULL);
-
- for (int i = 0; i < list->nSrc; ++i) {
- const char *name = list->a[i].zName;
- if (name != NULL && strcmp(new_name, name) == 0)
- return list;
- }
- struct Token token = { new_name, strlen(new_name), 0 };
- return sqlite3SrcListAppend(db, list, &token);
-}
-
+++ b/src/box/sql/select.c
@@ -225,6 +225,33 @@ findRightmost(Select * p)
return p;
}
+
+/**
+ * Work the same as sqlite3SrcListAppend(), but before adding to
+ * list provide check on name duplicates: only values with unique
+ * names are appended.
+ *
+ * @param db Database handler.
+ * @param list List of entries.
+ * @param new_name Name of entity to be added.
+ * @retval @list with new element on success, old one otherwise.
+ */
+static struct SrcList *
+sql_src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
+ const char *new_name)
+{
+ assert(list != NULL);
+ assert(new_name != NULL);
+
+ for (int i = 0; i < list->nSrc; ++i) {
+ const char *name = list->a[i].zName;
+ if (name != NULL && strcmp(new_name, name) == 0)
+ return list;
+ }
+ struct Token token = { new_name, strlen(new_name), 0 };
+ return sqlite3SrcListAppend(db, list, &token);
+}
+
>
>> +
>> +/**
>> + * Expand all spaces names from 'FROM' clause, including
>> + * ones from subqueries, and add those names to the original
>> + * select.
>> + *
>> + * @param select Select to be expanded.
>> + */
>> +void
>> +sql_select_expand_from_tables(struct Select *select);
>
> 9. If you agree with my suggestion to create sql_view_update_references(),
> you can move this function into select.c together with the former and
> make it static.
Actually, I can’t do so: sql_view_update_references() uses schema_find_id()
now, which in turn relies on cpp code.
>
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 28e4d7a4d..053cf83b7 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -2009,72 +2007,74 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
>> }
>> #ifndef SQLITE_OMIT_VIEW
>
> 10. Maybe it is worth to delete SQLITE_OMIT_VIEW alongside?
Ok, I will remove it in a separate patch.
>
>> -/*
>> - * The parser calls this routine in order to create a new VIEW
>> - */
>> void
>> -sqlite3CreateView(Parse * pParse, /* The parsing context */
>> - Token * pBegin, /* The CREATE token that begins the statement */
>> - Token * pName, /* The token that holds the name of the view */
>> - ExprList * pCNames, /* Optional list of view column names */
>> - Select * pSelect, /* A SELECT statement that will become the new view */
>> - int noErr /* Suppress error messages if VIEW already exists */
>> - )
>> +sql_create_view(struct Parse *parse_context, struct Token *begin,
>> + struct Token *name, struct ExprList *aliases,
>> + struct Select *select, bool if_exists)
>
> 11. 'if_exists' in build.c, but 'is_exists' in sqliteInt.h.
‘if_exists’ is surely to be more correct.
+++ b/src/box/sql/sqliteInt.h
@@ -3582,7 +3582,7 @@ int sqlite3FaultSim(int);
void
sql_create_view(struct Parse *parse_context, struct Token *begin,
struct Token *name, struct ExprList *aliases,
- struct Select *select, bool is_exists);
+ struct Select *select, bool if_exists);
> 12. Please, consider this diff:
Ok, simply applied.
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 053cf83b7..935054540 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2020,16 +2020,16 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
> }
> sqlite3StartTable(parse_context, name, if_exists);
> struct Table *p = parse_context->pNewTable;
> - if (p == NULL || parse_context->nErr)
> + if (p == NULL || parse_context->nErr != 0)
> goto create_view_fail;
> struct Table *sel_tab = sqlite3ResultSetOfSelect(parse_context, select);
> if (sel_tab == NULL)
> goto create_view_fail;
> if (aliases != NULL) {
> if ((int)sel_tab->def->field_count != aliases->nExpr) {
> - sqlite3ErrorMsg(parse_context,
> - "expected %d columns for '%s' but got %d",
> - aliases->nExpr, p->def->name,
> + sqlite3ErrorMsg(parse_context, "expected %d columns "\
> + "for '%s' but got %d", aliases->nExpr,
> + p->def->name,
> sel_tab->def->field_count);
> goto create_view_fail;
> }
> @@ -2053,21 +2053,17 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
> */
> struct Token end = parse_context->sLastToken;
> assert(end.z[0] != 0);
> - if (end.z[0] != ';') {
> + if (end.z[0] != ';')
> end.z += end.n;
> - }
> end.n = 0;
> - int n = (int)(end.z - begin->z);
> + int n = end.z - begin->z;
> assert(n > 0);
> const char *z = begin->z;
> - while (sqlite3Isspace(z[n - 1])) {
> + while (sqlite3Isspace(z[n - 1]))
> n--;
> - }
> end.z = &z[n - 1];
> end.n = 1;
> - p->def->opts.sql = malloc(n + 1);
> - memcpy(p->def->opts.sql, begin->z, n);
> - p->def->opts.sql[n] = '\0';
> + p->def->opts.sql = strndup(begin->z, n);
>
>> + p->def->opts.sql = malloc(n + 1);
>
> 13. No error checking. Here you should to set diag OOM, increment
> parser.nErr etc.
Fixed:
+ p->def->opts.sql = strndup(begin->z, n);
+ if (p->def->opts.sql == NULL) {
+ diag_set(OutOfMemory, n, "strndup", "opts.sql");
+ parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->nErr++;
+ goto create_view_fail;
+ }
>
>> @@ -2084,155 +2084,22 @@ sql_view_column_names(struct Parse *parse, struct Table *table)
>> {
>> assert(table != NULL);
>> assert(space_is_view(table));
>> - /* A positive nCol means the columns names for this view
>> - * are already known.
>> - */
>> - if (table->def->field_count > 0)
>> - return 0;
>> -
>> - /* A negative nCol is a special marker meaning that we are
>> - * currently trying to compute the column names. If we
>> - * enter this routine with a negative nCol, it means two
>> - * or more views form a loop, like this:
>> - *
>> - * CREATE VIEW one AS SELECT * FROM two;
>> - * CREATE VIEW two AS SELECT * FROM one;
>> - *
>> - * Actually, the error above is now caught prior to
>> - * reaching this point. But the following test is still
>> - * important as it does come up in the following:
>> - *
>> - * CREATE TABLE main.ex1(a);
>> - * CREATE TEMP VIEW ex1 AS SELECT a FROM ex1;
>> - * SELECT * FROM temp.ex1;
>> - */
>> - if ((int)table->def->field_count < 0) {
>> - sqlite3ErrorMsg(parse, "view %s is circularly defined",
>> - table->def->name);
>> + struct sqlite3 *db = parse->db;
>> + struct Select *select;
>> + if (sql_view_compile(db, table->def->opts.sql, &select) != 0) {
>> + diag_log();
>
> 14. You should not log each error. sql_view_column_names can be called
> by the parser only, so user will receive the error anyway. Log is needed
> when 1) the error is not logged itself, see LoggedError, 2) when it
> emerges with no possibility to be raised up to a user.
Got it, fixed (see comments below, I slightly changed this function):
@@ -2080,16 +2082,13 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
#endif /* SQLITE_OMIT_VIEW */
int
-sql_view_column_names(struct Parse *parse, struct Table *table)
+sql_view_assign_cursors(struct Parse *parse, const char *view_stmt)
{
- assert(table != NULL);
- assert(space_is_view(table));
+ assert(view_stmt != NULL);
struct sqlite3 *db = parse->db;
struct Select *select;
- if (sql_view_compile(db, table->def->opts.sql, &select) != 0) {
- diag_log();
+ if (sql_view_compile(db, view_stmt, &select) != 0)
return -1;
- }
>
>> /**
>> * Remove entries from the _sql_stat1 and _sql_stat4
>> @@ -2283,7 +2150,6 @@ static void
>> sql_code_drop_table(struct Parse *parse_context, struct space *space,
>> bool is_view)
>> {
>> - struct sqlite3 *db = parse_context->db;
>> struct Vdbe *v = sqlite3GetVdbe(parse_context);
>> assert(v != NULL);
>> /*
>> @@ -2313,6 +2179,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
>> int space_id_reg = ++parse_context->nMem;
>> int space_id = space->def->id;
>> sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
>> + sqlite3VdbeAddOp1(v, OP_CheckViewReferences, space_id_reg);
>
> 15. Why do you need separate opcode here? As far as I can understand
> you can rely on Tarantool core. SQL can just delete the view from
> _space, and in alter.cc the refs will be checked.
When we are dropping space with view references such as:
***
CREATE TABLE t1(id PRIMARY KEY, a);
CREATE INDEX i1 ON t1(a);
CREATE VIEW v1 AS SELECT * FROM t1;
DROP TABLE t1;
***
Drop of t1 surely fails, but before removing entry from _space,
we have to drop all secondary indexes and some system information.
Without this check, deletion from _space fails, but too late — secondary
indexes have been already dropped.
>
>> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
>> index 2aa35a114..0b64aee74 100644
>> --- a/src/box/sql/select.c
>> +++ b/src/box/sql/select.c
>> @@ -228,6 +225,43 @@ findRightmost(Select * p)
>> return p;
>> }
>> +/**
>> + * This function is an inner call of recursive traverse through
>> + * select AST starting from interface function
>> + * sql_select_expand_from_tables().
>> + *
>> + * @param top_select The root of AST.
>> + * @param sub_select sub-select of current level recursion.
>> + */
>> +static void
>> +expand_names_sub_select(struct Select *top_select,
>> + struct Select *sub_select)
>> +{
>
> 16. Out of 80 below, and declaration fits in one line. Please, consider this
> diff:
Ok, looks elegant. Applied.
>> +void
>> +sql_select_expand_from_tables(struct Select *select)
>> +{
>> + assert(select != NULL);
>> + for (int i = 0; i < select->pSrc->nSrc; ++i)
>> + if (select->pSrc->a[i].zName == NULL)
>> + expand_names_sub_select(select,
>> + select->pSrc->a[i].pSelect);
>> +}
>
> 17. Multi-line 'if' and 'for' must have {}:
>
> @@ -256,10 +255,11 @@ void
> sql_select_expand_from_tables(struct Select *select)
> {
> assert(select != NULL);
> - for (int i = 0; i < select->pSrc->nSrc; ++i)
> - if (select->pSrc->a[i].zName == NULL)
> - expand_names_sub_select(select,
> - select->pSrc->a[i].pSelect);
> + struct SrcList_item *src = select->pSrc->a;
> + for (int i = 0; i < select->pSrc->nSrc; ++i, ++src) {
> + if (src->zName == NULL)
> + expand_names_sub_select(select, src->pSelect);
> + }
> }
Fixed (just applied your diff).
>
>> @@ -4738,13 +4772,16 @@ selectExpander(Walker * pWalker, Select * p)
>> }
>> #if !defined(SQLITE_OMIT_VIEW)
>> if (space_is_view(pTab)) {
>> - if (sql_view_column_names(pParse, pTab) != 0)
>> + struct Select *select;
>> + if (sql_view_compile(db, pTab->def->opts.sql,
>> + &select) != 0) {
>> + diag_log();
>> return WRC_Abort;
>> + }
>> + sqlite3SrcListAssignCursors(pParse,
>> + select->pSrc);
>
> 18. Looks exactly like sql_view_column_names(). Why have not you used it?
Since this struct Select is used also in several places below.
Moreover, now I see that purpose of this function doesn’t
correspond to its name…I have renamed it to sql_view_assign_cursors():
+++ b/src/box/sql/sqliteInt.h
@@ -3582,20 +3582,19 @@ int sqlite3FaultSim(int);
void
sql_create_view(struct Parse *parse_context, struct Token *begin,
struct Token *name, struct ExprList *aliases,
- struct Select *select, bool is_exists);
+ struct Select *select, bool if_exists);
/**
- * The Table structure pTable is really a VIEW. Fill in the names
- * of the columns of the view in the table structure. Return the
- * number of errors. If an error is seen leave an error message
- * in parse->zErrMsg.
+ * Compile view, i.e. create struct Select from
+ * 'CREATE VIEW...' string, and assign cursors to each table from
+ * 'FROM' clause.
*
* @param parse Parsing context.
- * @param table Tables to process.
+ * @param view_stmt String containing 'CREATE VIEW' statement.
* @retval 0 if success, -1 in case of error.
*/
int
-sql_view_column_names(struct Parse *parse, struct Table *table);
+sql_view_assign_cursors(struct Parse *parse, const char *view_stmt);
@@ -2080,16 +2082,13 @@ sql_create_view(struct Parse *parse_context, struct Token *begin,
#endif /* SQLITE_OMIT_VIEW */
int
-sql_view_column_names(struct Parse *parse, struct Table *table)
+sql_view_assign_cursors(struct Parse *parse, const char *view_stmt)
{
- assert(table != NULL);
- assert(space_is_view(table));
+ assert(view_stmt != NULL);
struct sqlite3 *db = parse->db;
struct Select *select;
- if (sql_view_compile(db, table->def->opts.sql, &select) != 0) {
- diag_log();
+ if (sql_view_compile(db, view_stmt, &select) != 0)
return -1;
- }
sqlite3SrcListAssignCursors(parse, select->pSrc);
return 0;
}
>
>> assert(pFrom->pSelect == 0);
>> - assert(pTab->def->opts.is_view ==
>> - (pTab->pSelect != NULL));
>> - pFrom->pSelect =
>> - sqlite3SelectDup(db, pTab->pSelect, 0);
>> + pFrom->pSelect = select;
>> sqlite3SelectSetName(pFrom->pSelect,
>> pTab->def->name);
>> int columns = pTab->def->field_count;
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 01351a183..781f5d0a0 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -2962,6 +2959,8 @@ struct Parse {
>> bool parse_only;
>> /** If parse_only is set to true, store parsed expression. */
>> struct Expr *parsed_expr;
>> + /** If parse_only is set to true, store parsed SELECT. */
>> + struct Select *parsed_select;
>
> 19. Is it possible to make union of parsed_expr, parsed_select?
> Like this:
>
> union {
> struct Expr *expr;
> struct Select *select;
> } ast;
Yep, but also I have introduced simple AST type to tell
AST of SELECT from AST of Expr. Simple example which would
fail without type checking:
box.schema.create_space('view', {view = true})
sp = box.schema.create_space('test');
raw_sp = box.space._space:get(sp.id):totable();
sp:drop();
raw_sp[6].sql = 'SELECT 1';
raw_sp[6].view = true;
sp = box.space._space:replace(raw_sp);
In this case, parser really parses Expr with values 1.
It occurs due to wrapping Exprs in Selects...
Diff:
+++ b/src/box/sql/sqliteInt.h
@@ -2857,6 +2857,12 @@ struct TriggerPrg {
u32 aColmask[2]; /* Masks of old.*, new.* columns accessed */
};
+enum ast_type {
+ AST_TYPE_SELECT = 0,
+ AST_TYPE_EXPR,
+ ast_type_MAX
+};
+
/*
* An SQL parser context. A copy of this structure is passed through
* the parser and down into all the parser action routine in order to
@@ -2957,10 +2963,16 @@ struct Parse {
bool initiateTTrans; /* Initiate Tarantool transaction */
/** If set - do not emit byte code at all, just parse. */
bool parse_only;
- /** If parse_only is set to true, store parsed expression. */
- struct Expr *parsed_expr;
- /** If parse_only is set to true, store parsed SELECT. */
- struct Select *parsed_select;
+ /** Type of parsed_ast member. */
+ enum ast_type parsed_ast_type;
+ /**
+ * Members of this union are valid only
+ * if parse_only is set to true.
+ */
+ union {
+ struct Expr *expr;
+ struct Select *select;
+ } parsed_ast;
};
+++ b/src/box/sql/tokenize.c
@@ -558,11 +558,12 @@ sql_expr_compile(sqlite3 *db, const char *expr, int expr_len,
sprintf(stmt, "%s%.*s", outer, expr_len, expr);
char *unused;
- if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK) {
+ if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK ||
+ parser.parsed_ast_type != AST_TYPE_EXPR) {
diag_set(ClientError, ER_SQL_EXECUTE, expr);
return -1;
}
- *result = parser.parsed_expr;
+ *result = parser.parsed_ast.expr;
sql_parser_destroy(&parser);
return 0;
}
@@ -576,11 +577,13 @@ sql_view_compile(struct sqlite3 *db, const char *view_stmt,
sql_parser_create(&parser, db);
parser.parse_only = true;
char *unused;
- if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK) {
+ if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK ||
+ parser.parsed_ast_type != AST_TYPE_SELECT) {
diag_set(ClientError, ER_SQL_EXECUTE, view_stmt);
return -1;
}
- *select = parser.parsed_select;
+ *select = parser.parsed_ast.select;
@@ -6295,7 +6322,8 @@ sql_expr_extract_select(struct Parse *parser, struct Select *select)
{
struct ExprList *expr_list = select->pEList;
assert(expr_list->nExpr == 1);
- parser->parsed_expr = sqlite3ExprDup(parser->db,
- expr_list->a->pExpr,
- EXPRDUP_REDUCE);
+ parser->parsed_ast_type = AST_TYPE_EXPR;
+ parser->parsed_ast.expr = sqlite3ExprDup(parser->db,
+ expr_list->a->pExpr,
+ EXPRDUP_REDUCE);
@@ -2098,7 +2097,8 @@ void
sql_store_select(struct Parse *parse_context, struct Select *select)
{
Select *select_copy = sqlite3SelectDup(parse_context->db, select, 0);
- parse_context->parsed_select = select_copy;
+ parse_context->parsed_ast_type = AST_TYPE_SELECT;
+ parse_context->parsed_ast.select = select_copy;
}
+++ b/test/sql/view.test.lua
@@ -35,11 +35,17 @@ box.schema.create_space('view', {view = true})
sp = box.schema.create_space('test');
raw_sp = box.space._space:get(sp.id):totable();
sp:drop();
-raw_sp[6].sql = 'SELECT 1';
+raw_sp[6].sql = 'CREATE VIEW v as SELECT * FROM t1;';
raw_sp[6].view = true;
sp = box.space._space:replace(raw_sp);
box.space._space:select(sp['id'])[1]['name']
+-- Can't create view with incorrect SELECT statement.
+box.space.test:drop();
+-- This case must fail since parser converts it to expr AST.
+raw_sp[6].sql = 'SELECT 1;';
+sp = box.space._space:replace(raw_sp);
+
>
>> diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
>> index 42c70a255..25d8dc59b 100644
>> --- a/src/box/sql/tokenize.c
>> +++ b/src/box/sql/tokenize.c
>> @@ -566,3 +566,21 @@ sql_expr_compile(sqlite3 *db, const char *expr, int expr_len,
>> sql_parser_destroy(&parser);
>> return 0;
>> }
>> +
>> +int
>> +sql_view_compile(struct sqlite3 *db, const char *view_stmt,
>> + struct Select **select)
>> +{
>> + assert(select != NULL);
>> + struct Parse parser;
>> + sql_parser_create(&parser, db);
>> + parser.parse_only = true;
>> + char *unused;
>> + if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK) {
>> + diag_set(ClientError, ER_SQL_EXECUTE, view_stmt);
>
> 20. ER_SQL_EXECUTE expects parser error message in the argument, it is not?
> SQL statement in error looks useless. I see the fact of an error, but can not
> understand what is wrong.
/*157 */_(ER_SQL_EXECUTE, "Failed to execute SQL statement: %s") \
It takes one argument. Lets compare:
>tarantool SELECT * FROM v1;
Failed to execute SQL statement: CREATE VIEW v1 AS SELECT trash trash trash;
Failed to execute SQL statement: can’t compile SELECT AST from CREATE VIEW statement.
Is second one really more informative? To be honest, both seem to be satisfactory for me.
@@ -576,11 +577,14 @@ sql_view_compile(struct sqlite3 *db, const char *view_stmt,
sql_parser_create(&parser, db);
parser.parse_only = true;
char *unused;
- if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK) {
- diag_set(ClientError, ER_SQL_EXECUTE, view_stmt);
+ if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK ||
+ parser.parsed_ast_type != AST_TYPE_SELECT) {
+ diag_set(ClientError, ER_SQL_EXECUTE,
+ "can’t compile SELECT AST from CREATE VIEW statement");
+ sql_parser_destroy(&parser);
return -1;
}
>> + return -1;
>
> 21. Must not you destroy the parser regardless of result?
See diff above. Fixed.
>
>> + }
>> + *select = parser.parsed_select;
>> + sql_parser_destroy(&parser);
>> + return 0;
>> +}
>> diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
>> index ea3521133..32f40044f 100644
>> --- a/src/box/sql/trigger.c
>> +++ b/src/box/sql/trigger.c
>> @@ -758,7 +758,7 @@ codeTriggerProgram(Parse * pParse, /* The parser context */
>> sqlite3SelectDup(db, pStep->pSelect, 0);
>> sqlite3SelectDestInit(&sDest, SRT_Discard, 0);
>> sqlite3Select(pParse, pSelect, &sDest);
>> - sqlite3SelectDelete(db, pSelect);
>> + sql_select_delete(db, pSelect);
>
> 22. Something is wrong with indentation.
Fixed:
+++ b/src/box/sql/trigger.c
@@ -758,7 +758,7 @@ codeTriggerProgram(Parse * pParse, /* The parser context */
sqlite3SelectDup(db, pStep->pSelect, 0);
sqlite3SelectDestInit(&sDest, SRT_Discard, 0);
sqlite3Select(pParse, pSelect, &sDest);
- sql_select_delete(db, pSelect);
+ sql_select_delete(db, pSelect);
Since there are a lot of changes, I am attaching the whole patch below:
=======================================================================
Subject: [PATCH] sql: rework VIEW internals
This patch significantly reworks VIEW mechanisms.
Firstly, names resolution for VIEW now occurs during its creation.
In other words, we can't run following code, since name T1 doesn't exist
at the moment of V1 creation:
CREATE VIEW v1 AS SELECT * FROM t1;
CREATE TABLE t1(id PRIMARY KEY);
Now, table t1 must be created before view. As a result, no circularly
defined views are allowed. Moreover, it means that space representing
view is created with appropriate field names, types, collations etc.
Also, introduced view reference counter for each space. It is
incremented for each space participating in select statement.
For instace:
CREATE VIEW v1 AS SELECT * FROM (SELECT * FROM t1, t2);
In this case, view reference counter is increased for spaces T1 and T2.
Similarly, such counter is decremented for all dependent spaces when
VIEW is dropped. To support such behavior, auxiliary
on_commit triggers are added. However, it is still not enough, since
before dropping space itself, we must drop secondary indexes, clear
_sequence space etc. Such changes can't be rollbacked (due to the lack
of transactional DDL), so before executing DROP routine, special opcode
OP_CheckViewReferences checks view reference counter for space to be
dropped.
Finally, we don't hold struct Select in struct Table or anywhere else
anymore. Instead, 'CREATE VIEW AS SELECT ...' string is stored in
def->opts.sql. At compilation time of query on view
(such as 'SELECT * FROM v1;') this string is parsed once and loaded
into struct Select, which in turn is used as before.
Fixed tests where view was created prior to referenced table.
Closes #3429, #3368, #3300
---
src/box/alter.cc | 102 +++++++++++++++
src/box/space_def.c | 1 +
src/box/space_def.h | 2 +
src/box/sql.h | 53 ++++++++
src/box/sql/build.c | 283 +++++++++++------------------------------
src/box/sql/delete.c | 8 +-
src/box/sql/expr.c | 5 +-
src/box/sql/fkey.c | 4 +-
src/box/sql/insert.c | 7 +-
src/box/sql/parse.y | 19 +--
src/box/sql/pragma.c | 6 +-
src/box/sql/select.c | 109 +++++++++++++---
src/box/sql/sqliteInt.h | 58 +++++++--
src/box/sql/tokenize.c | 24 +++-
src/box/sql/trigger.c | 8 +-
src/box/sql/update.c | 3 +-
src/box/sql/vdbe.c | 24 ++++
test/sql-tap/colname.test.lua | 4 +-
test/sql-tap/drop_all.test.lua | 4 +-
test/sql-tap/fkey2.test.lua | 1 +
test/sql-tap/trigger1.test.lua | 1 +
test/sql-tap/triggerC.test.lua | 1 +
test/sql-tap/view.test.lua | 137 +++++++++++++++-----
test/sql/view.result | 60 ++++++++-
test/sql/view.test.lua | 28 +++-
25 files changed, 643 insertions(+), 309 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index b62f8adea..fe82006cf 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -944,6 +944,7 @@ ModifySpace::alter_def(struct alter_space *alter)
new_dict = new_def->dict;
new_def->dict = alter->old_space->def->dict;
tuple_dictionary_ref(new_def->dict);
+ new_def->view_ref_count = alter->old_space->def->view_ref_count;
space_def_delete(alter->space_def);
alter->space_def = new_def;
@@ -1440,6 +1441,73 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin,
}
}
+/**
+ * Walk through all spaces from 'FROM' clause of given select,
+ * and update their view references counters.
+ *
+ * @param select Tables from this select to be updated.
+ * @param update_value +1 on view creation, -1 on drop.
+ */
+static void
+update_view_references(struct Select *select, int update_value)
+{
+ assert(update_value == 1 || update_value == -1);
+ sql_select_expand_from_tables(select);
+ int from_tables_count = sql_select_from_tables_count(select);
+ for (int i = 0; i < from_tables_count; ++i) {
+ const char *space_name = sql_select_from_table_name(select, i);
+ if (space_name == NULL)
+ continue;
+ uint32_t space_id = schema_find_id(BOX_SPACE_ID, 2, space_name,
+ strlen(space_name));
+ assert(space_id != BOX_ID_NIL);
+ struct space *space = space_by_id(space_id);
+ assert(space->def->view_ref_count > 0 || update_value > 0);
+ space->def->view_ref_count += update_value;
+ }
+}
+
+/**
+ * Trigger which is fired on creation of new SQL view.
+ * Its purpose is to increment view reference counters of
+ * dependent spaces.
+ */
+static void
+on_create_view_commit(struct trigger *trigger, void *event)
+{
+ (void) event;
+ struct Select *select = (struct Select *)trigger->data;
+ update_view_references(select, 1);
+ sql_select_delete(sql_get(), select);
+}
+
+/**
+ * Trigger which is fired on drop of SQL view.
+ * Its purpose is to decrement view reference counters of
+ * dependent spaces.
+ */
+static void
+on_drop_view_commit(struct trigger *trigger, void *event)
+{
+ (void) event;
+ struct Select *select = (struct Select *)trigger->data;
+ update_view_references(select, -1);
+ sql_select_delete(sql_get(), select);
+}
+
+/**
+ * This trigger is invoked on drop/create of SQL view.
+ * Release memory for struct SELECT compiled in
+ * on_replace_dd_space trigger.
+ */
+static void
+on_alter_view_rollback(struct trigger *trigger, void *event)
+{
+ (void) event;
+ struct Select *select = (struct Select *)trigger->data;
+ sql_select_delete(sql_get(), select);
+}
+
/**
* A trigger which is invoked on replace in a data dictionary
* space _space.
@@ -1545,6 +1613,20 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_create_space_commit, space);
txn_on_commit(txn, on_commit);
+ if (def->opts.is_view) {
+ struct Select *select = sql_view_compile(sql_get(),
+ def->opts.sql);
+ if (select == NULL)
+ diag_raise();
+ struct trigger *on_commit_view =
+ txn_alter_trigger_new(on_create_view_commit,
+ select);
+ txn_on_commit(txn, on_commit_view);
+ struct trigger *on_rollback_view =
+ txn_alter_trigger_new(on_alter_view_rollback,
+ select);
+ txn_on_rollback(txn, on_rollback_view);
+ }
struct trigger *on_rollback =
txn_alter_trigger_new(on_create_space_rollback, space);
txn_on_rollback(txn, on_rollback);
@@ -1566,6 +1648,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
tnt_raise(ClientError, ER_DROP_SPACE,
space_name(old_space),
"the space has truncate record");
+ if (old_space->def->view_ref_count > 0) {
+ tnt_raise(ClientError, ER_DROP_SPACE,
+ space_name(old_space),
+ "other views depend on this space");
+ }
/**
* The space must be deleted from the space
* cache right away to achieve linearisable
@@ -1575,6 +1662,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct trigger *on_commit =
txn_alter_trigger_new(on_drop_space_commit, space);
txn_on_commit(txn, on_commit);
+ if (old_space->def->opts.is_view) {
+ struct Select *select =
+ sql_view_compile(sql_get(),
+ old_space->def->opts.sql);
+ if (select == NULL)
+ diag_raise();
+ struct trigger *on_commit_view =
+ txn_alter_trigger_new(on_drop_view_commit,
+ select);
+ txn_on_commit(txn, on_commit_view);
+ struct trigger *on_rollback_view =
+ txn_alter_trigger_new(on_alter_view_rollback,
+ select);
+ txn_on_rollback(txn, on_rollback_view);
+ }
struct trigger *on_rollback =
txn_alter_trigger_new(on_drop_space_rollback, space);
txn_on_rollback(txn, on_rollback);
diff --git a/src/box/space_def.c b/src/box/space_def.c
index a39978182..21b7d82bb 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -204,6 +204,7 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count,
memcpy(def->engine_name, engine_name, engine_len);
def->engine_name[engine_len] = 0;
+ def->view_ref_count = 0;
def->field_count = field_count;
if (field_count == 0) {
def->fields = NULL;
diff --git a/src/box/space_def.h b/src/box/space_def.h
index 024e868a9..4ff9c668e 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -107,6 +107,8 @@ struct space_def {
struct field_def *fields;
/** Length of @a fields. */
uint32_t field_count;
+ /** Number of SQL views which refer to this space. */
+ uint32_t view_ref_count;
struct space_opts opts;
char name[0];
};
diff --git a/src/box/sql.h b/src/box/sql.h
index 23021e56b..7de7c31fa 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -65,6 +65,7 @@ sql_get();
struct Expr;
struct Parse;
struct Select;
+struct SrcList;
struct Table;
/**
@@ -83,6 +84,17 @@ int
sql_expr_compile(struct sqlite3 *db, const char *expr, int expr_len,
struct Expr **result);
+/**
+ * This routine executes parser on 'CREATE VIEW ...' statement
+ * and loads content of SELECT into internal structs as result.
+ *
+ * @param db Current SQL context.
+ * @param view_stmt String containing 'CREATE VIEW' statement.
+ * @retval AST of SELECT statement on success, NULL otherwise.
+ */
+struct Select *
+sql_view_compile(struct sqlite3 *db, const char *view_stmt);
+
/**
* Store duplicate of a parsed expression into @a parser.
* @param parser Parser context.
@@ -293,6 +305,47 @@ sql_parser_create(struct Parse *parser, struct sqlite3 *db);
void
sql_parser_destroy(struct Parse *parser);
+/**
+ * Release memory allocated for given SELECT and all of its
+ * substructures. It accepts NULL pointers.
+ *
+ * @param db Database handler.
+ * @param select Select to be freed.
+ */
+void
+sql_select_delete(struct sqlite3 *db, struct Select *select);
+
+/**
+ * Expand all spaces names from 'FROM' clause, including
+ * ones from subqueries, and add those names to the original
+ * select.
+ *
+ * @param select Select to be expanded.
+ */
+void
+sql_select_expand_from_tables(struct Select *select);
+
+/**
+ * Temporary getter in order to avoid including sqliteInt.h
+ * in alter.cc.
+ *
+ * @param select Select to be examined.
+ * @retval Count of 'FROM' tables in given select.
+ */
+int
+sql_select_from_tables_count(const struct Select *select);
+
+/**
+ * Temporary getter in order to avoid including sqliteInt.h
+ * in alter.cc.
+ *
+ * @param select Select to be examined.
+ * @param i Ordinal number of 'FROM' table.
+ * @retval Name of i-th 'FROM' table.
+ */
+const char *
+sql_select_from_table_name(const struct Select *select, int i);
+
#if defined(__cplusplus)
} /* extern "C" { */
#endif
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 28e4d7a4d..53bb53ab8 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -405,7 +405,6 @@ deleteTable(sqlite3 * db, Table * pTable)
sqlite3HashClear(&pTable->idxHash);
sqlite3DbFree(db, pTable->aCol);
sqlite3DbFree(db, pTable->zColAff);
- sqlite3SelectDelete(db, pTable->pSelect);
assert(pTable->def != NULL);
/* Do not delete pTable->def allocated on region. */
if (!pTable->def->opts.temporary)
@@ -1849,7 +1848,6 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
p->def->id = SQLITE_PAGENO_TO_SPACEID(p->tnum);
}
- assert(p->def->opts.is_view == (p->pSelect != NULL));
if (!p->def->opts.is_view) {
if ((p->tabFlags & TF_HasPrimaryKey) == 0) {
sqlite3ErrorMsg(pParse,
@@ -2009,230 +2007,99 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
#ifndef SQLITE_OMIT_VIEW
-/*
- * The parser calls this routine in order to create a new VIEW
- */
void
-sqlite3CreateView(Parse * pParse, /* The parsing context */
- Token * pBegin, /* The CREATE token that begins the statement */
- Token * pName, /* The token that holds the name of the view */
- ExprList * pCNames, /* Optional list of view column names */
- Select * pSelect, /* A SELECT statement that will become the new view */
- int noErr /* Suppress error messages if VIEW already exists */
- )
+sql_create_view(struct Parse *parse_context, struct Token *begin,
+ struct Token *name, struct ExprList *aliases,
+ struct Select *select, bool if_exists)
{
- Table *p;
- int n;
- const char *z;
- Token sEnd;
- DbFixer sFix;
- sqlite3 *db = pParse->db;
-
- if (pParse->nVar > 0) {
- sqlite3ErrorMsg(pParse, "parameters are not allowed in views");
+ struct sqlite3 *db = parse_context->db;
+ if (parse_context->nVar > 0) {
+ sqlite3ErrorMsg(parse_context,
+ "parameters are not allowed in views");
goto create_view_fail;
}
- sqlite3StartTable(pParse, pName, noErr);
- p = pParse->pNewTable;
- if (p == 0 || pParse->nErr)
+ sqlite3StartTable(parse_context, name, if_exists);
+ struct Table *p = parse_context->pNewTable;
+ if (p == NULL || parse_context->nErr != 0)
goto create_view_fail;
- sqlite3FixInit(&sFix, pParse, "view", pName);
- if (sqlite3FixSelect(&sFix, pSelect))
+ struct Table *sel_tab = sqlite3ResultSetOfSelect(parse_context, select);
+ if (sel_tab == NULL)
goto create_view_fail;
-
- /* Make a copy of the entire SELECT statement that defines the view.
- * This will force all the Expr.token.z values to be dynamically
- * allocated rather than point to the input string - which means that
- * they will persist after the current sqlite3_exec() call returns.
- */
- p->pSelect = sqlite3SelectDup(db, pSelect, EXPRDUP_REDUCE);
+ if (aliases != NULL) {
+ if ((int)sel_tab->def->field_count != aliases->nExpr) {
+ sqlite3ErrorMsg(parse_context, "expected %d columns "\
+ "for '%s' but got %d", aliases->nExpr,
+ p->def->name,
+ sel_tab->def->field_count);
+ goto create_view_fail;
+ }
+ sqlite3ColumnsFromExprList(parse_context, aliases, p);
+ sqlite3SelectAddColumnTypeAndCollation(parse_context, p,
+ select);
+ } else {
+ assert(p->aCol == NULL);
+ assert(sel_tab->def->opts.temporary);
+ p->def->fields = sel_tab->def->fields;
+ p->def->field_count = sel_tab->def->field_count;
+ p->aCol = sel_tab->aCol;
+ sel_tab->aCol = NULL;
+ sel_tab->def->fields = NULL;
+ sel_tab->def->field_count = 0;
+ }
p->def->opts.is_view = true;
- p->def->opts.checks = sql_expr_list_dup(db, pCNames, EXPRDUP_REDUCE);
- if (db->mallocFailed)
- goto create_view_fail;
-
- /* Locate the end of the CREATE VIEW statement. Make sEnd point to
- * the end.
+ /*
+ * Locate the end of the CREATE VIEW statement.
+ * Make sEnd point to the end.
*/
- sEnd = pParse->sLastToken;
- assert(sEnd.z[0] != 0);
- if (sEnd.z[0] != ';') {
- sEnd.z += sEnd.n;
- }
- sEnd.n = 0;
- n = (int)(sEnd.z - pBegin->z);
+ struct Token end = parse_context->sLastToken;
+ assert(end.z[0] != 0);
+ if (end.z[0] != ';')
+ end.z += end.n;
+ end.n = 0;
+ int n = end.z - begin->z;
assert(n > 0);
- z = pBegin->z;
- while (sqlite3Isspace(z[n - 1])) {
+ const char *z = begin->z;
+ while (sqlite3Isspace(z[n - 1]))
n--;
+ end.z = &z[n - 1];
+ end.n = 1;
+ p->def->opts.sql = strndup(begin->z, n);
+ if (p->def->opts.sql == NULL) {
+ diag_set(OutOfMemory, n, "strndup", "opts.sql");
+ parse_context->rc = SQL_TARANTOOL_ERROR;
+ parse_context->nErr++;
+ goto create_view_fail;
}
- sEnd.z = &z[n - 1];
- sEnd.n = 1;
/* Use sqlite3EndTable() to add the view to the Tarantool. */
- sqlite3EndTable(pParse, 0, &sEnd, 0);
+ sqlite3EndTable(parse_context, 0, &end, 0);
create_view_fail:
- sqlite3SelectDelete(db, pSelect);
- sql_expr_list_delete(db, pCNames);
+ sql_expr_list_delete(db, aliases);
+ sql_select_delete(db, select);
return;
}
#endif /* SQLITE_OMIT_VIEW */
int
-sql_view_column_names(struct Parse *parse, struct Table *table)
+sql_view_assign_cursors(struct Parse *parse, const char *view_stmt)
{
- assert(table != NULL);
- assert(space_is_view(table));
- /* A positive nCol means the columns names for this view
- * are already known.
- */
- if (table->def->field_count > 0)
- return 0;
-
- /* A negative nCol is a special marker meaning that we are
- * currently trying to compute the column names. If we
- * enter this routine with a negative nCol, it means two
- * or more views form a loop, like this:
- *
- * CREATE VIEW one AS SELECT * FROM two;
- * CREATE VIEW two AS SELECT * FROM one;
- *
- * Actually, the error above is now caught prior to
- * reaching this point. But the following test is still
- * important as it does come up in the following:
- *
- * CREATE TABLE main.ex1(a);
- * CREATE TEMP VIEW ex1 AS SELECT a FROM ex1;
- * SELECT * FROM temp.ex1;
- */
- if ((int)table->def->field_count < 0) {
- sqlite3ErrorMsg(parse, "view %s is circularly defined",
- table->def->name);
- return -1;
- }
-
- /* If we get this far, it means we need to compute the
- * table names. Note that the call to
- * sqlite3ResultSetOfSelect() will expand any "*" elements
- * in the results set of the view and will assign cursors
- * to the elements of the FROM clause. But we do not want
- * these changes to be permanent. So the computation is
- * done on a copy of the SELECT statement that defines the
- * view.
- */
- assert(table->pSelect != NULL);
- sqlite3 *db = parse->db;
- struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
+ assert(view_stmt != NULL);
+ struct sqlite3 *db = parse->db;
+ struct Select *select = sql_view_compile(db, view_stmt);
if (select == NULL)
return -1;
- int n = parse->nTab;
sqlite3SrcListAssignCursors(parse, select->pSrc);
- table->def->field_count = -1;
- db->lookaside.bDisable++;
- struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
- parse->nTab = n;
- int rc = 0;
- /* Get server checks. */
- ExprList *checks = space_checks_expr_list(table->def->id);
- if (checks != NULL) {
- /* CREATE VIEW name(arglist) AS ...
- * The names of the columns in the table are taken
- * from arglist which is stored in table->pCheck.
- * The pCheck field normally holds CHECK
- * constraints on an ordinary table, but for a
- * VIEW it holds the list of column names.
- */
- struct space_def *old_def = table->def;
- sqlite3ColumnsFromExprList(parse, checks, table);
- if (sql_table_def_rebuild(db, table) != 0) {
- rc = -1;
- } else {
- space_def_delete(old_def);
- }
- if (db->mallocFailed == 0 && parse->nErr == 0
- && (int)table->def->field_count == select->pEList->nExpr) {
- sqlite3SelectAddColumnTypeAndCollation(parse, table,
- select);
- }
- } else if (sel_tab != NULL) {
- /* CREATE VIEW name AS... without an argument
- * list. Construct the column names from the
- * SELECT statement that defines the view.
- */
- assert(table->aCol == NULL);
- assert(sel_tab->def->opts.temporary);
- struct space_def *old_def = table->def;
- struct space_def *new_def =
- sql_ephemeral_space_def_new(parse,
- old_def->name);
- if (new_def == NULL) {
- rc = -1;
- } else {
- memcpy(new_def, old_def,
- sizeof(struct space_def));
- new_def->dict = NULL;
- new_def->opts.temporary = true;
- new_def->fields = sel_tab->def->fields;
- new_def->field_count =
- sel_tab->def->field_count;
- table->def = new_def;
- if (sql_table_def_rebuild(db, table) != 0)
- rc = -1;
- }
- table->aCol = sel_tab->aCol;
- sel_tab->aCol = NULL;
- sel_tab->def = old_def;
- sel_tab->def->fields = NULL;
- sel_tab->def->field_count = 0;
- } else {
- table->def->field_count = 0;
- rc = -1;
- }
- sqlite3DeleteTable(db, sel_tab);
- sqlite3SelectDelete(db, select);
- db->lookaside.bDisable--;
-
- return rc;
+ return 0;
}
-#ifndef SQLITE_OMIT_VIEW
-/*
- * Clear the column names from every VIEW in database idx.
- */
-static void
-sqliteViewResetAll(sqlite3 * db)
+void
+sql_store_select(struct Parse *parse_context, struct Select *select)
{
- HashElem *i;
- for (i = sqliteHashFirst(&db->pSchema->tblHash); i;
- i = sqliteHashNext(i)) {
- Table *pTab = sqliteHashData(i);
- assert(pTab->def->opts.is_view == (pTab->pSelect != NULL));
- if (pTab->def->opts.is_view) {
- sqlite3DbFree(db, pTab->aCol);
- struct space_def *old_def = pTab->def;
- assert(old_def->opts.temporary == false);
- pTab->def = space_def_new(old_def->id, old_def->uid,
- 0, old_def->name,
- strlen(old_def->name),
- old_def->engine_name,
- strlen(old_def->engine_name),
- &old_def->opts,
- NULL, 0);
- if (pTab->def == NULL) {
- sqlite3OomFault(db);
- pTab->def = old_def;
- } else {
- space_def_delete(old_def);
- }
- pTab->aCol = NULL;
- pTab->def->field_count = 0;
- }
- }
+ Select *select_copy = sqlite3SelectDup(parse_context->db, select, 0);
+ parse_context->parsed_ast_type = AST_TYPE_SELECT;
+ parse_context->parsed_ast.select = select_copy;
}
-#else
-#define sqliteViewResetAll(A,B)
-#endif /* SQLITE_OMIT_VIEW */
/**
* Remove entries from the _sql_stat1 and _sql_stat4
@@ -2283,7 +2150,6 @@ static void
sql_code_drop_table(struct Parse *parse_context, struct space *space,
bool is_view)
{
- struct sqlite3 *db = parse_context->db;
struct Vdbe *v = sqlite3GetVdbe(parse_context);
assert(v != NULL);
/*
@@ -2313,6 +2179,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
int space_id_reg = ++parse_context->nMem;
int space_id = space->def->id;
sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
+ sqlite3VdbeAddOp1(v, OP_CheckViewReferences, space_id_reg);
if (space->sequence != NULL) {
/* Delete entry from _space_sequence. */
sqlite3VdbeAddOp3(v, OP_MakeRecord, space_id_reg, 1,
@@ -2371,14 +2238,6 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
VdbeComment((v, "Delete entry from _space"));
/* Remove the table entry from SQLite's internal schema. */
sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, space->def->name, 0);
-
- /*
- * Replace to _space/_index will fail if active
- * transaction. So, don't pretend, that we are able to
- * anything back. To be fixed when transactions
- * DDL are enabled.
- */
- sqliteViewResetAll(db);
}
/**
@@ -2914,7 +2773,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
assert(pTab != 0);
assert(parse->nErr == 0);
#ifndef SQLITE_OMIT_VIEW
- if (pTab->pSelect) {
+ if (pTab->def->opts.is_view) {
sqlite3ErrorMsg(parse, "views may not be indexed");
goto exit_create_index;
}
@@ -3680,7 +3539,7 @@ sqlite3SrcListDelete(sqlite3 * db, SrcList * pList)
if (pItem->fg.isTabFunc)
sql_expr_list_delete(db, pItem->u1.pFuncArg);
sqlite3DeleteTable(db, pItem->pTab);
- sqlite3SelectDelete(db, pItem->pSelect);
+ sql_select_delete(db, pItem->pSelect);
sql_expr_delete(db, pItem->pOn, false);
sqlite3IdListDelete(db, pItem->pUsing);
}
@@ -3739,7 +3598,7 @@ sqlite3SrcListAppendFromTerm(Parse * pParse, /* Parsing context */
assert(p == 0);
sql_expr_delete(db, pOn, false);
sqlite3IdListDelete(db, pUsing);
- sqlite3SelectDelete(db, pSubquery);
+ sql_select_delete(db, pSubquery);
return 0;
}
@@ -4141,7 +4000,7 @@ sqlite3WithAdd(Parse * pParse, /* Parsing context */
if (db->mallocFailed) {
sql_expr_list_delete(db, pArglist);
- sqlite3SelectDelete(db, pQuery);
+ sql_select_delete(db, pQuery);
sqlite3DbFree(db, zName);
pNew = pWith;
} else {
@@ -4166,7 +4025,7 @@ sqlite3WithDelete(sqlite3 * db, With * pWith)
for (i = 0; i < pWith->nCte; i++) {
struct Cte *pCte = &pWith->a[i];
sql_expr_list_delete(db, pCte->pCols);
- sqlite3SelectDelete(db, pCte->pSelect);
+ sql_select_delete(db, pCte->pSelect);
sqlite3DbFree(db, pCte->zName);
}
sqlite3DbFree(db, pWith);
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index ddad54b3e..318511f69 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -68,7 +68,7 @@ sql_materialize_view(struct Parse *parse, const char *name, struct Expr *where,
struct SelectDest dest;
sqlite3SelectDestInit(&dest, SRT_EphemTab, cursor);
sqlite3Select(parse, select, &dest);
- sqlite3SelectDelete(db, select);
+ sql_select_delete(db, select);
}
void
@@ -123,7 +123,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* initialized.
*/
if (is_view) {
- if (sql_view_column_names(parse, table) != 0)
+ if (sql_view_assign_cursors(parse, table->def->opts.sql) != 0)
goto delete_from_cleanup;
if (trigger_list == NULL) {
@@ -339,7 +339,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (one_pass != ONEPASS_OFF) {
/* OP_Found will use an unpacked key. */
assert(key_len == pk_len);
- assert(pk != NULL || table->pSelect != NULL);
+ assert(pk != NULL || table->def->opts.is_view);
sqlite3VdbeAddOp4Int(v, OP_NotFound, tab_cursor,
addr_bypass, reg_key, key_len);
@@ -483,7 +483,7 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
* of the DELETE statement is to fire the INSTEAD OF
* triggers).
*/
- if (table->pSelect == NULL) {
+ if (!table->def->opts.is_view) {
uint8_t p5 = 0;
sqlite3VdbeAddOp2(v, OP_Delete, cursor,
(need_update_count ? OPFLAG_NCHANGE : 0));
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 8866f6fed..a0475fbf3 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -953,7 +953,7 @@ sqlite3PExprAddSelect(Parse * pParse, Expr * pExpr, Select * pSelect)
sqlite3ExprSetHeightAndFlags(pParse, pExpr);
} else {
assert(pParse->db->mallocFailed);
- sqlite3SelectDelete(pParse->db, pSelect);
+ sql_select_delete(pParse->db, pSelect);
}
}
@@ -1152,7 +1152,7 @@ sqlite3ExprDeleteNN(sqlite3 * db, Expr * p, bool extern_alloc)
if (!extern_alloc)
sql_expr_delete(db, p->pRight, extern_alloc);
if (ExprHasProperty(p, EP_xIsSelect)) {
- sqlite3SelectDelete(db, p->x.pSelect);
+ sql_select_delete(db, p->x.pSelect);
} else {
sql_expr_list_delete(db, p->x.pList);
}
@@ -2191,7 +2191,6 @@ isCandidateForInOpt(Expr * pX)
return 0; /* FROM is not a subquery or view */
pTab = pSrc->a[0].pTab;
assert(pTab != 0);
- assert(pTab->def->opts.is_view == (pTab->pSelect != NULL));
/* FROM clause is not a view */
assert(!pTab->def->opts.is_view);
pEList = p->pEList;
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 70ebef89f..2644a26f9 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -744,7 +744,7 @@ fkTriggerDelete(sqlite3 * dbMem, Trigger * p)
TriggerStep *pStep = p->step_list;
sql_expr_delete(dbMem, pStep->pWhere, false);
sql_expr_list_delete(dbMem, pStep->pExprList);
- sqlite3SelectDelete(dbMem, pStep->pSelect);
+ sql_select_delete(dbMem, pStep->pSelect);
sql_expr_delete(dbMem, p->pWhen, false);
sqlite3DbFree(dbMem, p);
}
@@ -1418,7 +1418,7 @@ fkActionTrigger(Parse * pParse, /* Parse context */
sql_expr_delete(db, pWhere, false);
sql_expr_delete(db, pWhen, false);
sql_expr_list_delete(db, pList);
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
if (db->mallocFailed == 1) {
fkTriggerDelete(db, pTrigger);
return 0;
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 59c61c703..041cfbec7 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -371,7 +371,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
&& pSelect->pPrior == 0) {
pList = pSelect->pEList;
pSelect->pEList = 0;
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
pSelect = 0;
}
@@ -398,7 +398,8 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* If pTab is really a view, make sure it has been initialized.
* ViewGetColumnNames() is a no-op if pTab is not a view.
*/
- if (is_view && sql_view_column_names(pParse, pTab) != 0)
+ if (is_view &&
+ sql_view_assign_cursors(pParse, pTab->def->opts.sql) != 0)
goto insert_cleanup;
/* Cannot insert into a read-only table. */
@@ -920,7 +921,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
insert_cleanup:
sqlite3SrcListDelete(db, pTabList);
sql_expr_list_delete(db, pList);
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
sqlite3IdListDelete(db, pColumn);
sqlite3DbFree(db, aRegIdx);
}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 2e5f349c8..fc5d2f0a6 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -184,7 +184,7 @@ create_table_args ::= LP columnlist conslist_opt(X) RP(E). {
}
create_table_args ::= AS select(S). {
sqlite3EndTable(pParse,0,0,S);
- sqlite3SelectDelete(pParse->db, S);
+ sql_select_delete(pParse->db, S);
}
columnlist ::= columnlist COMMA columnname carglist.
columnlist ::= columnname carglist.
@@ -373,7 +373,10 @@ ifexists(A) ::= . {A = 0;}
%ifndef SQLITE_OMIT_VIEW
cmd ::= createkw(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C)
AS select(S). {
- sqlite3CreateView(pParse, &X, &Y, C, S, E);
+ if (!pParse->parse_only)
+ sql_create_view(pParse, &X, &Y, C, S, E);
+ else
+ sql_store_select(pParse, S);
}
cmd ::= DROP VIEW ifexists(E) fullname(X). {
sql_drop_table(pParse, X, 1, E);
@@ -388,15 +391,15 @@ cmd ::= select(X). {
sqlite3Select(pParse, X, &dest);
else
sql_expr_extract_select(pParse, X);
- sqlite3SelectDelete(pParse->db, X);
+ sql_select_delete(pParse->db, X);
}
%type select {Select*}
-%destructor select {sqlite3SelectDelete(pParse->db, $$);}
+%destructor select {sql_select_delete(pParse->db, $$);}
%type selectnowith {Select*}
-%destructor selectnowith {sqlite3SelectDelete(pParse->db, $$);}
+%destructor selectnowith {sql_select_delete(pParse->db, $$);}
%type oneselect {Select*}
-%destructor oneselect {sqlite3SelectDelete(pParse->db, $$);}
+%destructor oneselect {sql_select_delete(pParse->db, $$);}
%include {
/*
@@ -453,7 +456,7 @@ selectnowith(A) ::= selectnowith(A) multiselect_op(Y) oneselect(Z). {
pRhs->selFlags &= ~SF_MultiValue;
if( Y!=TK_ALL ) pParse->hasCompound = 1;
}else{
- sqlite3SelectDelete(pParse->db, pLhs);
+ sql_select_delete(pParse->db, pLhs);
}
A = pRhs;
}
@@ -496,7 +499,7 @@ oneselect(A) ::= SELECT(S) distinct(D) selcollist(W) from(X) where_opt(Y)
oneselect(A) ::= values(A).
%type values {Select*}
-%destructor values {sqlite3SelectDelete(pParse->db, $$);}
+%destructor values {sql_select_delete(pParse->db, $$);}
values(A) ::= VALUES LP nexprlist(X) RP. {
A = sqlite3SelectNew(pParse,X,0,0,0,0,0,SF_Values,0,0);
}
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 9dab5a7fd..d6ec0f222 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -357,8 +357,10 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Column *pCol;
Index *pPk = sqlite3PrimaryKeyIndex(pTab);
pParse->nMem = 6;
- if (space_is_view(pTab))
- sql_view_column_names(pParse, pTab);
+ if (space_is_view(pTab)) {
+ const char *sql = pTab->def->opts.sql;
+ sql_view_assign_cursors(pParse, sql);
+ }
for (i = 0, pCol = pTab->aCol;
i < (int)pTab->def->field_count;
i++, pCol++) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 2aa35a114..b2ddf442f 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -207,16 +207,28 @@ sqlite3SelectSetName(Select * p, const char *zName)
}
#endif
-/*
- * Delete the given Select structure and all of its substructures.
- */
void
-sqlite3SelectDelete(sqlite3 * db, Select * p)
+sql_select_delete(sqlite3 *db, Select *p)
{
if (p)
clearSelect(db, p, 1);
}
+int
+sql_select_from_tables_count(const struct Select *select)
+{
+ assert(select != NULL && select->pSrc != NULL);
+ return select->pSrc->nSrc;
+}
+
+const char *
+sql_select_from_table_name(const struct Select *select, int i)
+{
+ assert(select != NULL && select->pSrc != NULL);
+ assert(i >= 0 && i < select->pSrc->nSrc);
+ return select->pSrc->a[i].zName;
+}
+
/*
* Return a pointer to the right-most SELECT statement in a compound.
*/
@@ -228,6 +240,70 @@ findRightmost(Select * p)
return p;
}
+
+/**
+ * Work the same as sqlite3SrcListAppend(), but before adding to
+ * list provide check on name duplicates: only values with unique
+ * names are appended.
+ *
+ * @param db Database handler.
+ * @param list List of entries.
+ * @param new_name Name of entity to be added.
+ * @retval @list with new element on success, old one otherwise.
+ */
+static struct SrcList *
+src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
+ const char *new_name)
+{
+ assert(list != NULL);
+ assert(new_name != NULL);
+
+ for (int i = 0; i < list->nSrc; ++i) {
+ const char *name = list->a[i].zName;
+ if (name != NULL && strcmp(new_name, name) == 0)
+ return list;
+ }
+ struct Token token = { new_name, strlen(new_name), 0 };
+ return sqlite3SrcListAppend(db, list, &token);
+}
+
+/**
+ * This function is an inner call of recursive traverse through
+ * select AST starting from interface function
+ * sql_select_expand_from_tables().
+ *
+ * @param top_select The root of AST.
+ * @param sub_select sub-select of current level recursion.
+ */
+static void
+expand_names_sub_select(struct Select *top_select, struct Select *sub_select)
+{
+ assert(top_select != NULL);
+ assert(sub_select != NULL);
+ struct SrcList_item *sub_src = sub_select->pSrc->a;
+ for (int i = 0; i < sub_select->pSrc->nSrc; ++i, ++sub_src) {
+ if (sub_src->zName == NULL) {
+ expand_names_sub_select(top_select, sub_src->pSelect);
+ } else {
+ top_select->pSrc =
+ src_list_append_unique(sql_get(),
+ top_select->pSrc,
+ sub_src->zName);
+ }
+ }
+}
+
+void
+sql_select_expand_from_tables(struct Select *select)
+{
+ assert(select != NULL);
+ struct SrcList_item *src = select->pSrc->a;
+ for (int i = 0; i < select->pSrc->nSrc; ++i, ++src) {
+ if (select->pSrc->a[i].zName == NULL)
+ expand_names_sub_select(select, src->pSelect);
+ }
+}
+
/*
* Given 1 to 3 identifiers preceding the JOIN keyword, determine the
* type of join. Return an integer constant that expresses that type
@@ -2830,7 +2906,7 @@ multiSelect(Parse * pParse, /* Parsing context */
multi_select_end:
pDest->iSdst = dest.iSdst;
pDest->nSdst = dest.nSdst;
- sqlite3SelectDelete(db, pDelete);
+ sql_select_delete(db, pDelete);
return rc;
}
#endif /* SQLITE_OMIT_COMPOUND_SELECT */
@@ -3432,7 +3508,7 @@ multiSelectOrderBy(Parse * pParse, /* Parsing context */
* by the calling function
*/
if (p->pPrior) {
- sqlite3SelectDelete(db, p->pPrior);
+ sql_select_delete(db, p->pPrior);
}
p->pPrior = pPrior;
pPrior->pNext = p;
@@ -4106,7 +4182,7 @@ flattenSubquery(Parse * pParse, /* Parsing context */
/* Finially, delete what is left of the subquery and return
* success.
*/
- sqlite3SelectDelete(db, pSub1);
+ sql_select_delete(db, pSub1);
#ifdef SELECTTRACE_ENABLED
if (sqlite3SelectTrace & 0x100) {
@@ -4738,13 +4814,15 @@ selectExpander(Walker * pWalker, Select * p)
}
#if !defined(SQLITE_OMIT_VIEW)
if (space_is_view(pTab)) {
- if (sql_view_column_names(pParse, pTab) != 0)
+ struct Select *select =
+ sql_view_compile(db,
+ pTab->def->opts.sql);
+ if (select == NULL)
return WRC_Abort;
+ sqlite3SrcListAssignCursors(pParse,
+ select->pSrc);
assert(pFrom->pSelect == 0);
- assert(pTab->def->opts.is_view ==
- (pTab->pSelect != NULL));
- pFrom->pSelect =
- sqlite3SelectDup(db, pTab->pSelect, 0);
+ pFrom->pSelect = select;
sqlite3SelectSetName(pFrom->pSelect,
pTab->def->name);
int columns = pTab->def->field_count;
@@ -6258,7 +6336,8 @@ sql_expr_extract_select(struct Parse *parser, struct Select *select)
{
struct ExprList *expr_list = select->pEList;
assert(expr_list->nExpr == 1);
- parser->parsed_expr = sqlite3ExprDup(parser->db,
- expr_list->a->pExpr,
- EXPRDUP_REDUCE);
+ parser->parsed_ast_type = AST_TYPE_EXPR;
+ parser->parsed_ast.expr = sqlite3ExprDup(parser->db,
+ expr_list->a->pExpr,
+ EXPRDUP_REDUCE);
}
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 01351a183..0f5fb0ad3 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1913,7 +1913,6 @@ struct Column {
struct Table {
Column *aCol; /* Information about each column */
Index *pIndex; /* List of SQL indexes on this table. */
- Select *pSelect; /* NULL for tables. Points to definition if a view. */
FKey *pFKey; /* Linked list of all foreign keys in this table */
char *zColAff; /* String defining the affinity of each column */
/* ... also used as column name list in a VIEW */
@@ -2688,9 +2687,7 @@ struct Select {
LogEst nSelectRow; /* Estimated number of result rows */
u32 selFlags; /* Various SF_* values */
int iLimit, iOffset; /* Memory registers holding LIMIT & OFFSET counters */
-#ifdef SELECTTRACE_ENABLED
char zSelName[12]; /* Symbolic name of this SELECT use for debugging */
-#endif
int addrOpenEphm[2]; /* OP_OpenEphem opcodes related to this select */
SrcList *pSrc; /* The FROM clause */
Expr *pWhere; /* The WHERE clause */
@@ -2860,6 +2857,12 @@ struct TriggerPrg {
u32 aColmask[2]; /* Masks of old.*, new.* columns accessed */
};
+enum ast_type {
+ AST_TYPE_SELECT = 0,
+ AST_TYPE_EXPR,
+ ast_type_MAX
+};
+
/*
* An SQL parser context. A copy of this structure is passed through
* the parser and down into all the parser action routine in order to
@@ -2960,8 +2963,16 @@ struct Parse {
bool initiateTTrans; /* Initiate Tarantool transaction */
/** If set - do not emit byte code at all, just parse. */
bool parse_only;
- /** If parse_only is set to true, store parsed expression. */
- struct Expr *parsed_expr;
+ /** Type of parsed_ast member. */
+ enum ast_type parsed_ast_type;
+ /**
+ * Members of this union are valid only
+ * if parse_only is set to true.
+ */
+ union {
+ struct Expr *expr;
+ struct Select *select;
+ } parsed_ast;
};
/*
@@ -3570,20 +3581,42 @@ int sqlite3ParseUri(const char *, const char *, unsigned int *,
int sqlite3FaultSim(int);
#endif
-void sqlite3CreateView(Parse *, Token *, Token *, ExprList *, Select *, int);
+/**
+ * The parser calls this routine in order to create a new VIEW.
+ *
+ * @param parse_context Current parsing context.
+ * @param begin The CREATE token that begins the statement.
+ * @param name The token that holds the name of the view.
+ * @param aliases Optional list of view column names.
+ * @param select A SELECT statement that will become the new view.
+ * @param if_exists Suppress error messages if VIEW already exists.
+ */
+void
+sql_create_view(struct Parse *parse_context, struct Token *begin,
+ struct Token *name, struct ExprList *aliases,
+ struct Select *select, bool if_exists);
/**
- * The Table structure pTable is really a VIEW. Fill in the names
- * of the columns of the view in the table structure. Return the
- * number of errors. If an error is seen leave an error message
- * in parse->zErrMsg.
+ * Compile view, i.e. create struct Select from
+ * 'CREATE VIEW...' string, and assign cursors to each table from
+ * 'FROM' clause.
*
* @param parse Parsing context.
- * @param table Tables to process.
+ * @param view_stmt String containing 'CREATE VIEW' statement.
* @retval 0 if success, -1 in case of error.
*/
int
-sql_view_column_names(struct Parse *parse, struct Table *table);
+sql_view_assign_cursors(struct Parse *parse, const char *view_stmt);
+
+/**
+ * Store duplicate of SELECT into parsing context.
+ * This routine is called during parsing.
+ *
+ * @param parse_context Current parsing context.
+ * @param select Select to be stored.
+ */
+void
+sql_store_select(struct Parse *parse_context, struct Select *select);
void
sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
@@ -3656,7 +3689,6 @@ sql_drop_index(struct Parse *, struct SrcList *, struct Token *, bool);
int sqlite3Select(Parse *, Select *, SelectDest *);
Select *sqlite3SelectNew(Parse *, ExprList *, SrcList *, Expr *, ExprList *,
Expr *, ExprList *, u32, Expr *, Expr *);
-void sqlite3SelectDelete(sqlite3 *, Select *);
/**
* While a SrcList can in general represent multiple tables and
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 42c70a255..b08034cec 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -558,11 +558,31 @@ sql_expr_compile(sqlite3 *db, const char *expr, int expr_len,
sprintf(stmt, "%s%.*s", outer, expr_len, expr);
char *unused;
- if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK) {
+ if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK ||
+ parser.parsed_ast_type != AST_TYPE_EXPR) {
diag_set(ClientError, ER_SQL_EXECUTE, expr);
return -1;
}
- *result = parser.parsed_expr;
+ *result = parser.parsed_ast.expr;
sql_parser_destroy(&parser);
return 0;
}
+
+struct Select *
+sql_view_compile(struct sqlite3 *db, const char *view_stmt)
+{
+ struct Parse parser;
+ sql_parser_create(&parser, db);
+ parser.parse_only = true;
+ char *unused;
+ if (sqlite3RunParser(&parser, view_stmt, &unused) != SQLITE_OK ||
+ parser.parsed_ast_type != AST_TYPE_SELECT) {
+ diag_set(ClientError, ER_SQL_EXECUTE,
+ "can’t compile SELECT AST from CREATE VIEW statement");
+ sql_parser_destroy(&parser);
+ return NULL;
+ }
+ struct Select *result = parser.parsed_ast.select;
+ sql_parser_destroy(&parser);
+ return result;
+}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index ea3521133..fea9c9d35 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -54,7 +54,7 @@ sqlite3DeleteTriggerStep(sqlite3 * db, TriggerStep * pTriggerStep)
sql_expr_delete(db, pTmp->pWhere, false);
sql_expr_list_delete(db, pTmp->pExprList);
- sqlite3SelectDelete(db, pTmp->pSelect);
+ sql_select_delete(db, pTmp->pSelect);
sqlite3IdListDelete(db, pTmp->pIdList);
sqlite3DbFree(db, pTmp);
@@ -346,7 +346,7 @@ sqlite3TriggerSelectStep(sqlite3 * db, Select * pSelect)
TriggerStep *pTriggerStep =
sqlite3DbMallocZero(db, sizeof(TriggerStep));
if (pTriggerStep == 0) {
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
return 0;
}
pTriggerStep->op = TK_SELECT;
@@ -412,7 +412,7 @@ sqlite3TriggerInsertStep(sqlite3 * db, /* The database connection */
} else {
sqlite3IdListDelete(db, pColumn);
}
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
return pTriggerStep;
}
@@ -758,7 +758,7 @@ codeTriggerProgram(Parse * pParse, /* The parser context */
sqlite3SelectDup(db, pStep->pSelect, 0);
sqlite3SelectDestInit(&sDest, SRT_Discard, 0);
sqlite3Select(pParse, pSelect, &sDest);
- sqlite3SelectDelete(db, pSelect);
+ sql_select_delete(db, pSelect);
break;
}
}
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 590aad28b..8890b077b 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -140,7 +140,8 @@ sqlite3Update(Parse * pParse, /* The parser context */
is_view = space_is_view(pTab);
assert(pTrigger || tmask == 0);
- if (is_view && sql_view_column_names(pParse, pTab) != 0) {
+ if (is_view &&
+ sql_view_assign_cursors(pParse,pTab->def->opts.sql) != 0) {
goto update_cleanup;
}
if (is_view && tmask == 0) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 3fe5875ca..5b5cf835d 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2983,6 +2983,30 @@ case OP_FkCheckCommit: {
break;
}
+/* Opcode: CheckViewReferences P1 * * * *
+ * Synopsis: r[P1] = space id
+ *
+ * Check that space to be dropped doesn't have any view
+ * references. This opcode is needed since Tarantool lacks
+ * DDL transaction. On the other hand, to drop space, we must
+ * firstly drop secondary indexes from _index system space,
+ * clear _truncate table etc.
+ */
+case OP_CheckViewReferences: {
+ assert(pOp->p1 > 0);
+ pIn1 = &aMem[pOp->p1];
+ uint32_t space_id = pIn1->u.i;
+ struct space *space = space_by_id(space_id);
+ assert(space != NULL);
+ if (space->def->view_ref_count > 0) {
+ sqlite3VdbeError(p,"Can't drop table %s: other views depend "
+ "on this space", space->def->name);
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
+ }
+ break;
+}
+
/* Opcode: TransactionBegin * * * * *
*
* Start Tarantool's transaction.
diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua
index c96623e4e..c53a1e885 100755
--- a/test/sql-tap/colname.test.lua
+++ b/test/sql-tap/colname.test.lua
@@ -623,14 +623,14 @@ end
test:do_test(
"colname-11.0.1",
function ()
- test:drop_all_tables()
+ test:drop_all_views()
end,
nil)
test:do_test(
"colname-11.0.2",
function ()
- test:drop_all_views()
+ test:drop_all_tables()
end,
nil)
diff --git a/test/sql-tap/drop_all.test.lua b/test/sql-tap/drop_all.test.lua
index b9abe7594..d4f6c6073 100755
--- a/test/sql-tap/drop_all.test.lua
+++ b/test/sql-tap/drop_all.test.lua
@@ -22,7 +22,7 @@ test:do_test(
test:do_test(
prefix.."-1.1",
function()
- return #test:drop_all_tables()
+ return #test:drop_all_views()
end,
N
)
@@ -30,7 +30,7 @@ test:do_test(
test:do_test(
prefix.."-1.1",
function()
- return #test:drop_all_views()
+ return #test:drop_all_tables()
end,
N
)
diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua
index f90c218d4..1f3a81d92 100755
--- a/test/sql-tap/fkey2.test.lua
+++ b/test/sql-tap/fkey2.test.lua
@@ -743,6 +743,7 @@ test:do_catchsql_test(
test:do_catchsql_test(
"fkey2-7.3",
[[
+ DROP VIEW v;
DROP TABLE IF EXISTS c;
CREATE TABLE p(a COLLATE binary, b PRIMARY KEY);
CREATE UNIQUE INDEX idx ON p(a COLLATE "unicode_ci");
diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 40daba4d8..dab3281a3 100755
--- a/test/sql-tap/trigger1.test.lua
+++ b/test/sql-tap/trigger1.test.lua
@@ -460,6 +460,7 @@ test:do_catchsql_test(
-- }
test:execsql [[
CREATE TABLE t2(x int PRIMARY KEY,y);
+ DROP VIEW v1;
DROP TABLE t1;
INSERT INTO t2 VALUES(3, 4);
INSERT INTO t2 VALUES(7, 8);
diff --git a/test/sql-tap/triggerC.test.lua b/test/sql-tap/triggerC.test.lua
index 347007832..e58072e2f 100755
--- a/test/sql-tap/triggerC.test.lua
+++ b/test/sql-tap/triggerC.test.lua
@@ -1097,6 +1097,7 @@ test:do_catchsql_test(
--
SQL = [[
DROP TABLE IF EXISTS t1;
+ DROP VIEW v2;
DROP TABLE IF EXISTS t2;
DROP TABLE IF EXISTS t4;
DROP TABLE IF EXISTS t5;
diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua
index 8ba2044f0..c24e3fb29 100755
--- a/test/sql-tap/view.test.lua
+++ b/test/sql-tap/view.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(65)
+test:plan(74)
--!./tcltestrunner.lua
-- 2002 February 26
@@ -125,13 +125,23 @@ test:do_catchsql_test(
"view-1.6",
[[
DROP TABLE t1;
- SELECT * FROM v1 ORDER BY a;
]], {
-- <view-1.6>
- 1, "no such table: T1"
+ 1, "Can't drop table T1: other views depend on this space"
-- </view-1.6>
})
+test:do_catchsql_test(
+ "view-1.7",
+ [[
+ DROP VIEW v1;
+ DROP TABLE t1;
+ ]], {
+ -- <view-1.7>
+ 0
+ -- </view-1.7>
+ })
+
-- ORIGINAL_TEST
-- do_test view-1.7 {
-- execsql {
@@ -143,12 +153,13 @@ test:do_catchsql_test(
-- }
-- } {2 3 5 6 8 9}
test:do_execsql_test(
- "view-1.7",
+ "view-1.8",
[[
CREATE TABLE t1(x primary key,a,b,c);
INSERT INTO t1 VALUES(1,2,3,4);
INSERT INTO t1 VALUES(4,5,6,7);
INSERT INTO t1 VALUES(7,8,9,10);
+ CREATE VIEW v1 AS SELECT a,b FROM t1;
SELECT * FROM v1 ORDER BY a;
]], {
-- <view-1.7>
@@ -156,24 +167,6 @@ test:do_execsql_test(
-- </view-1.7>
})
--- MUST_WORK_TEST reopen db
-if (0 > 0)
- then
- test:do_test(
- "view-1.8",
- function()
- db("close")
- sqlite3("db", "test.db")
- return test:execsql [[
- SELECT * FROM v1 ORDER BY a;
- ]]
- end, {
- -- <view-1.8>
- 2, 3, 5, 6, 8, 9
- -- </view-1.8>
- })
-
-end
test:do_test(
"view-2.1",
function()
@@ -1065,10 +1058,9 @@ end
test:do_execsql_test(
"view-20.1",
[[
- DROP TABLE IF EXISTS t1;
- DROP VIEW IF EXISTS v1;
- CREATE TABLE t1(c1 primary key);
- CREATE VIEW v1 AS SELECT c1 FROM (SELECT t1.c1 FROM t1);
+ DROP VIEW v10;
+ CREATE TABLE t10(c1 primary key);
+ CREATE VIEW v10 AS SELECT c1 FROM (SELECT t10.c1 FROM t10);
]], {
-- <view-20.1>
@@ -1078,10 +1070,10 @@ test:do_execsql_test(
test:do_execsql_test(
"view-20.1",
[[
- DROP TABLE IF EXISTS t1;
- DROP VIEW IF EXISTS v1;
- CREATE TABLE t1(c1 primary key);
- CREATE VIEW v1 AS SELECT c1 FROM (SELECT t1.c1 FROM t1);
+ DROP VIEW IF EXISTS v10;
+ DROP TABLE IF EXISTS t10;
+ CREATE TABLE t10(c1 primary key);
+ CREATE VIEW v10 AS SELECT c1 FROM (SELECT t10.c1 FROM t10);
]], {
-- <view-20.1>
@@ -1171,5 +1163,90 @@ if (0 > 0)
end
+-- Make sure that VIEW with several internal selects works.
+test:do_catchsql_test(
+ "view-23.1",
+ [[
+ CREATE TABLE t11(a INT PRIMARY KEY);
+ CREATE TABLE t12(b INT PRIMARY KEY);
+ CREATE TABLE t13(c INT PRIMARY KEY);
+ CREATE VIEW v11 AS SELECT * FROM
+ (SELECT a FROM (SELECT a, b FROM t11, t12)),
+ (SELECT * FROM (SELECT a, c FROM t11, t13));
+ ]], {
+ -- <view-23.1>
+ 0
+ -- </view-23.1>
+ })
+
+test:do_catchsql_test(
+ "view-23.2",
+ [[
+ DROP TABLE t11;
+ ]], {
+ -- <view-23.2>
+ 1, "Can't drop table T11: other views depend on this space"
+ -- </view-23.2>
+ })
+
+test:do_catchsql_test(
+ "view-23.3",
+ [[
+ DROP TABLE t12;
+ ]], {
+ -- <view-23.3>
+ 1, "Can't drop table T12: other views depend on this space"
+ -- </view-23.3>
+ })
+
+test:do_catchsql_test(
+ "view-23.4",
+ [[
+ DROP TABLE t13;
+ ]], {
+ -- <view-23.4>
+ 1, "Can't drop table T13: other views depend on this space"
+ -- </view-23.4>
+ })
+
+test:do_catchsql_test(
+ "view-23.5",
+ [[
+ DROP VIEW v11;
+ ]], {
+ -- <view-23.5>
+ 0
+ -- </view-23.5>
+ })
+
+test:do_catchsql_test(
+ "view-23.6",
+ [[
+ DROP TABLE t11;
+ ]], {
+ -- <view-23.6>
+ 0
+ -- </view-23.6>
+ })
+
+test:do_catchsql_test(
+ "view-23.7",
+ [[
+ DROP TABLE t12;
+ ]], {
+ -- <view-23.7>
+ 0
+ -- </view-23.7>
+ })
+
+test:do_catchsql_test(
+ "view-23.8",
+ [[
+ DROP TABLE t13;
+ ]], {
+ -- <view-23.8>
+ 0
+ -- </view-23.8>
+ })
test:finish_test()
diff --git a/test/sql/view.result b/test/sql/view.result
index 04cb3fae8..2b90e058a 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -73,7 +73,7 @@ raw_sp = box.space._space:get(sp.id):totable();
sp:drop();
---
...
-raw_sp[6].sql = 'fake';
+raw_sp[6].sql = 'CREATE VIEW v as SELECT * FROM t1;';
---
...
raw_sp[6].view = true;
@@ -86,13 +86,67 @@ box.space._space:select(sp['id'])[1]['name']
---
- test
...
--- Cleanup
+-- Can't create view with incorrect SELECT statement.
box.space.test:drop();
---
...
-box.sql.execute("DROP TABLE t1;");
+-- This case must fail since parser converts it to expr AST.
+raw_sp[6].sql = 'SELECT 1;';
+---
+...
+sp = box.space._space:replace(raw_sp);
+---
+- error: 'Failed to execute SQL statement: can’t compile SELECT AST from CREATE VIEW
+ statement'
+...
+-- Can't drop space via Lua if at least one view refers to it.
+box.sql.execute('CREATE TABLE t2(id INT PRIMARY KEY);');
+---
+...
+box.sql.execute('CREATE VIEW v2 AS SELECT * FROM t2;');
---
...
+box.space.T2:drop();
+---
+- error: 'Can''t drop space ''T2'': other views depend on this space'
+...
+box.sql.execute('DROP VIEW v2;');
+---
+...
+box.sql.execute('DROP TABLE t2;');
+---
+...
+-- Check that alter transfers reference counter.
+box.sql.execute("CREATE TABLE t2(id INTEGER PRIMARY KEY);");
+---
+...
+box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;");
+---
+...
+box.sql.execute("DROP TABLE t2;");
+---
+- error: 'Can''t drop table T2: other views depend on this space'
+...
+sp = box.space._space:get{box.space.T2.id};
+---
+...
+sp = box.space._space:replace(sp);
+---
+...
+box.sql.execute("DROP TABLE t2;");
+---
+- error: 'Can''t drop table T2: other views depend on this space'
+...
+box.sql.execute("DROP VIEW v2;");
+---
+...
+box.sql.execute("DROP TABLE t2;");
+---
+...
+-- Cleanup
box.sql.execute("DROP VIEW v1;");
---
...
+box.sql.execute("DROP TABLE t1;");
+---
+...
diff --git a/test/sql/view.test.lua b/test/sql/view.test.lua
index ab2843cb6..27f449f58 100644
--- a/test/sql/view.test.lua
+++ b/test/sql/view.test.lua
@@ -35,12 +35,34 @@ box.schema.create_space('view', {view = true})
sp = box.schema.create_space('test');
raw_sp = box.space._space:get(sp.id):totable();
sp:drop();
-raw_sp[6].sql = 'fake';
+raw_sp[6].sql = 'CREATE VIEW v as SELECT * FROM t1;';
raw_sp[6].view = true;
sp = box.space._space:replace(raw_sp);
box.space._space:select(sp['id'])[1]['name']
--- Cleanup
+-- Can't create view with incorrect SELECT statement.
box.space.test:drop();
-box.sql.execute("DROP TABLE t1;");
+-- This case must fail since parser converts it to expr AST.
+raw_sp[6].sql = 'SELECT 1;';
+sp = box.space._space:replace(raw_sp);
+
+-- Can't drop space via Lua if at least one view refers to it.
+box.sql.execute('CREATE TABLE t2(id INT PRIMARY KEY);');
+box.sql.execute('CREATE VIEW v2 AS SELECT * FROM t2;');
+box.space.T2:drop();
+box.sql.execute('DROP VIEW v2;');
+box.sql.execute('DROP TABLE t2;');
+
+-- Check that alter transfers reference counter.
+box.sql.execute("CREATE TABLE t2(id INTEGER PRIMARY KEY);");
+box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;");
+box.sql.execute("DROP TABLE t2;");
+sp = box.space._space:get{box.space.T2.id};
+sp = box.space._space:replace(sp);
+box.sql.execute("DROP TABLE t2;");
+box.sql.execute("DROP VIEW v2;");
+box.sql.execute("DROP TABLE t2;");
+
+-- Cleanup
box.sql.execute("DROP VIEW v1;");
+box.sql.execute("DROP TABLE t1;");
--
2.15.1
next prev parent reply other threads:[~2018-06-06 14:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-04 16:26 [tarantool-patches] " Nikita Pettik
2018-06-05 11:30 ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-06 14:25 ` n.pettik [this message]
2018-06-07 10:40 ` Vladislav Shpilevoy
2018-06-07 18:25 ` n.pettik
2018-06-07 20:06 ` Vladislav Shpilevoy
2018-06-08 13:17 ` n.pettik
2018-06-08 20:05 ` Vladislav Shpilevoy
2018-06-19 13:04 ` Kirill Yukhin
2018-06-08 4:08 ` Konstantin Osipov
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=C32B3E1B-2CBD-4CB7-8505-34DEF2096FAC@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.org \
--subject='[tarantool-patches] Re: [PATCH] sql: rework VIEW internals' \
/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