* [tarantool-patches] [PATCH] sql: refactor SQL delete to allow Lua spaces
@ 2018-05-18 13:36 Kirill Yukhin
2018-05-18 21:07 ` [tarantool-patches] " Vladislav Shpilevoy
0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2018-05-18 13:36 UTC (permalink / raw)
To: v.shpilevoy; +Cc: tarantool-patches, Kirill Yukhin
Branch: https://github.com/tarantool/tarantool/tree/kyukhin/gh-3235-sql-truncate-on-lua-spaces
Issue: https://github.com/tarantool/tarantool/issues/3235
This is a first step toward fully-featured deletion
of spaces created in Lua by means of SQL language.
This change to handle most simple case:
DELETE * FROM <space_name> and will be improved in
nearest future.
Part of #3235
---
src/box/sql/analyze.c | 3 +-
src/box/sql/build.c | 3 +-
src/box/sql/delete.c | 69 ++++++++++++++++++++++++++-----------------
src/box/sql/vdbe.c | 2 +-
test/sql-tap/delete1.test.lua | 23 ++++++++++++++-
5 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 998dd5b..cc594a7 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -169,7 +169,8 @@ openStatTable(Parse * pParse, /* Parsing context */
* The sql_stat[134] table already exists.
* Delete all rows.
*/
- sqlite3VdbeAddOp2(v, OP_Clear, aRoot[i], 0);
+ sqlite3VdbeAddOp2(v, OP_Clear,
+ SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
}
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index df75186..49e59f4 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2672,7 +2672,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
VdbeCoverage(v);
sqlite3VdbeJumpHere(v, addr1);
if (memRootPage < 0)
- sqlite3VdbeAddOp2(v, OP_Clear, tnum, 0);
+ sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
+ 0);
emit_open_cursor(pParse, iIdx, tnum);
sqlite3VdbeChangeP5(v,
OPFLAG_BULKCSR | ((memRootPage >= 0) ?
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f4d248e..dfc91cb 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
+#include "box/box.h"
#include "box/session.h"
#include "box/schema.h"
#include "sqliteInt.h"
@@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* with multiple tables and expect an SrcList* parameter
* instead of just a Table* parameter.
*/
- struct Table *table = sql_list_lookup_table(parse, tab_list);
- if (table == NULL)
- goto delete_from_cleanup;
-
- struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(table->tnum));
- assert(space != NULL);
+ struct Table *table = NULL;
+ struct space *space;
+ uint32_t space_id;
/* Figure out if we have any triggers and if the table
* being deleted from is a view.
*/
- struct Trigger *trigger_list = sqlite3TriggersExist(table, TK_DELETE,
- NULL, NULL);
-
- bool is_view = space->def->opts.is_view;
+ struct Trigger *trigger_list = NULL;
/* True if there are triggers or FKs or subqueries in the
* WHERE clause.
*/
- bool is_complex = trigger_list != NULL ||
- sqlite3FkRequired(table, NULL);
+ bool is_complex = false;
+ const char *tab_name = tab_list->a->zName;
+ if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
+ space_id = box_space_id_by_name(tab_name,
+ strlen(tab_name));
+ if (space_id == BOX_ID_NIL)
+ goto delete_from_cleanup;
+ } else {
+ table = sql_list_lookup_table(parse, tab_list);
+ if (table == NULL)
+ goto delete_from_cleanup;
+ space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+ trigger_list =sqlite3TriggersExist(table, TK_DELETE,
+ NULL, NULL);
+ is_complex = trigger_list != NULL ||
+ sqlite3FkRequired(table, NULL);
+ }
+ space = space_by_id(space_id);
+ assert(space != NULL);
+
+ bool is_view = space->def->opts.is_view;
/* If table is really a view, make sure it has been
* initialized.
*/
- if (sqlite3ViewGetColumnNames(parse, table))
- goto delete_from_cleanup;
+ if (is_view) {
+ if (sqlite3ViewGetColumnNames(parse, table))
+ goto delete_from_cleanup;
- if (is_view && trigger_list == NULL) {
- sqlite3ErrorMsg(parse, "cannot modify %s because it is a view",
- space->def->name);
- goto delete_from_cleanup;
+ if (trigger_list == NULL) {
+ sqlite3ErrorMsg(parse, "cannot modify %s because it is a"
+ " view", space->def->name);
+ goto delete_from_cleanup;
+ }
}
/* Assign cursor numbers to the table and all its indices.
@@ -139,14 +155,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
tab_cursor);
}
- /* Resolve the column names in the WHERE clause. */
- struct NameContext nc;
- memset(&nc, 0, sizeof(nc));
- nc.pParse = parse;
- nc.pSrcList = tab_list;
- if (sqlite3ResolveExprNames(&nc, where))
- goto delete_from_cleanup;
-
/* Initialize the counter of the number of rows deleted,
* if we are counting rows.
*/
@@ -162,7 +170,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (where == NULL && !is_complex) {
assert(!is_view);
- sqlite3VdbeAddOp1(v, OP_Clear, table->tnum);
+ sqlite3VdbeAddOp1(v, OP_Clear, space_id);
/* Do not start Tarantool's transaction in case of
* truncate optimization. This is workaround until
@@ -171,6 +179,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
*/
parse->initiateTTrans = false;
} else {
+ /* Resolve the column names in the WHERE clause. */
+ struct NameContext nc;
+ memset(&nc, 0, sizeof(nc));
+ nc.pParse = parse;
+ nc.pSrcList = tab_list;
+ if (sqlite3ResolveExprNames(&nc, where))
+ goto delete_from_cleanup;
uint16_t wcf = WHERE_ONEPASS_DESIRED | WHERE_DUPLICATES_OK |
WHERE_SEEK_TABLE;
if (nc.ncFlags & NC_VarSelect)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 455198d..9acb9b6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4614,7 +4614,7 @@ case OP_IdxGE: { /* jump */
*/
case OP_Clear: {
assert(pOp->p1 > 0);
- uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pOp->p1);
+ uint32_t space_id = pOp->p1;
struct space *space = space_by_id(space_id);
assert(space != NULL);
rc = tarantoolSqlite3ClearTable(space);
diff --git a/test/sql-tap/delete1.test.lua b/test/sql-tap/delete1.test.lua
index 61d7756..810ca8a 100755
--- a/test/sql-tap/delete1.test.lua
+++ b/test/sql-tap/delete1.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(7)
+test:plan(9)
--!./tcltestrunner.lua
-- ["set","testdir",[["file","dirname",["argv0"]]]]
@@ -131,5 +131,26 @@ test:do_test(
-- </delete1-5.0>
})
+-- Tests for data dictionary integration.
+s = box.schema.create_space('t')
+i = s:create_index('i', {parts={1, 'unsigned'}})
+test:do_test(
+ "delete1-6.0",
+ function()
+ s:replace({1})
+ s:replace({2})
+ s:replace({3})
+ return s:count()
+ end,
+ 3)
+
+test:do_test(
+ "delete1-6.1",
+ function()
+ box.sql.execute([[delete from "t"]])
+ return s:count()
+ end,
+ 0)
+
test:finish_test()
--
2.16.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-18 13:36 [tarantool-patches] [PATCH] sql: refactor SQL delete to allow Lua spaces Kirill Yukhin
@ 2018-05-18 21:07 ` Vladislav Shpilevoy
2018-05-21 8:20 ` Kirill Yukhin
0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-18 21:07 UTC (permalink / raw)
To: tarantool-patches, Kirill Yukhin
Hello. Thanks for the patch! See 1 comment below.
On 18/05/2018 16:36, Kirill Yukhin wrote:
> Branch: https://github.com/tarantool/tarantool/tree/kyukhin/gh-3235-sql-truncate-on-lua-spaces
> Issue: https://github.com/tarantool/tarantool/issues/3235
>
> This is a first step toward fully-featured deletion
> of spaces created in Lua by means of SQL language.
> This change to handle most simple case:
> DELETE * FROM <space_name> and will be improved in
> nearest future.
>
> Part of #3235
> ---
> src/box/sql/analyze.c | 3 +-
> src/box/sql/build.c | 3 +-
> src/box/sql/delete.c | 69 ++++++++++++++++++++++++++-----------------
> src/box/sql/vdbe.c | 2 +-
> test/sql-tap/delete1.test.lua | 23 ++++++++++++++-
> 5 files changed, 69 insertions(+), 31 deletions(-)
>
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index f4d248e..dfc91cb 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -29,6 +29,7 @@
> * SUCH DAMAGE.
> */
>
> +#include "box/box.h"
> #include "box/session.h"
> #include "box/schema.h"
> #include "sqliteInt.h"
> @@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> + bool is_complex = false;
> + const char *tab_name = tab_list->a->zName;
> + if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
> + space_id = box_space_id_by_name(tab_name,
> + strlen(tab_name));
> + if (space_id == BOX_ID_NIL)
> + goto delete_from_cleanup;
> + } else {
> + table = sql_list_lookup_table(parse, tab_list);
> + if (table == NULL)
> + goto delete_from_cleanup;
> + space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
> + trigger_list =sqlite3TriggersExist(table, TK_DELETE,
> + NULL, NULL);
> + is_complex = trigger_list != NULL ||
> + sqlite3FkRequired(table, NULL);
> + }
> + space = space_by_id(space_id);
> + assert(space != NULL);
> +
> + bool is_view = space->def->opts.is_view;
>
> /* If table is really a view, make sure it has been
> * initialized.
> */
> - if (sqlite3ViewGetColumnNames(parse, table))
> - goto delete_from_cleanup;
> + if (is_view) {
1. sqlite3ViewGetColumnNames makes the check too. Lets turn it into assertion
inside sqlite3ViewGetColumnNames, and add the 'if (is_view)' in other places,
where it is called and the check is necessary. For example, in selectExpander
it is checked twice too.
> + if (sqlite3ViewGetColumnNames(parse, table))
> + goto delete_from_cleanup;
>
>
> /* Assign cursor numbers to the table and all its indices.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-18 21:07 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-05-21 8:20 ` Kirill Yukhin
2018-05-21 9:47 ` Vladislav Shpilevoy
0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2018-05-21 8:20 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
Hi Vlad,
Thanks a lot for review.
My comment is inlined. Updated patch in the bottom.
On 19 мая 00:07, Vladislav Shpilevoy wrote:
> Hello. Thanks for the patch! See 1 comment below.
>
> On 18/05/2018 16:36, Kirill Yukhin wrote:
> > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> > @@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> > + bool is_complex = false;
> > + const char *tab_name = tab_list->a->zName;
> > + if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
> > + space_id = box_space_id_by_name(tab_name,
> > + strlen(tab_name));
> > + if (space_id == BOX_ID_NIL)
> > + goto delete_from_cleanup;
> > + } else {
> > + table = sql_list_lookup_table(parse, tab_list);
> > + if (table == NULL)
> > + goto delete_from_cleanup;
> > + space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
> > + trigger_list =sqlite3TriggersExist(table, TK_DELETE,
> > + NULL, NULL);
> > + is_complex = trigger_list != NULL ||
> > + sqlite3FkRequired(table, NULL);
> > + }
> > + space = space_by_id(space_id);
> > + assert(space != NULL);
> > +
> > + bool is_view = space->def->opts.is_view;
> > /* If table is really a view, make sure it has been
> > * initialized.
> > */
> > - if (sqlite3ViewGetColumnNames(parse, table))
> > - goto delete_from_cleanup;
> > + if (is_view) {
>
> 1. sqlite3ViewGetColumnNames makes the check too. Lets turn it into assertion
> inside sqlite3ViewGetColumnNames, and add the 'if (is_view)' in other places,
> where it is called and the check is necessary. For example, in selectExpander
> it is checked twice too.
I've put explicit assert and refactored sqlite3ViewGetColumnNames() to obey
Tarantool's coding style. I've also put explicit check at other call sites of
the function.
--
Regards, Kirill Yukhin
commit ea717170e89a01881526bf3b546af6dffa273de1
Author: Kirill Yukhin <kyukhin@tarantool.org>
Date: Fri May 18 16:31:24 2018 +0300
sql: refactor SQL delete to allow Lua spaces
This is a first step toward fully-featured deletion
of spaces created in Lua by means of SQL language.
This change to handle most simple case:
DELETE * FROM <space_name> and will be improved in
nearest future.
Also, refactor (sqlite3ViewGetColumnNames()) to
use Tarantool's code style and naming convention.
Part of #3235
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 998dd5b..cc594a7 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -169,7 +169,8 @@ openStatTable(Parse * pParse, /* Parsing context */
* The sql_stat[134] table already exists.
* Delete all rows.
*/
- sqlite3VdbeAddOp2(v, OP_Clear, aRoot[i], 0);
+ sqlite3VdbeAddOp2(v, OP_Clear,
+ SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
}
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index df75186..68f81e8 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
}
#endif /* SQLITE_OMIT_VIEW */
-#if !defined(SQLITE_OMIT_VIEW)
-/*
- * The Table structure pTable is really a VIEW. Fill in the names of
- * the columns of the view in the pTable structure. Return the number
- * of errors. If an error is seen leave an error message in pParse->zErrMsg.
- */
-int
-sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
+bool
+sql_view_column_names(struct Parse *parse, struct Table *table)
{
- Table *pSelTab; /* A fake table from which we get the result set */
- Select *pSel; /* Copy of the SELECT that implements the view */
- int nErr = 0; /* Number of errors encountered */
- int n; /* Temporarily holds the number of cursors assigned */
- sqlite3 *db = pParse->db; /* Database connection for malloc errors */
-
- assert(pTable);
-
-#ifndef SQLITE_OMIT_VIEW
- /* A positive nCol means the columns names for this view are
- * already known.
+ assert(table != NULL);
+ assert(space_is_view(table));
+ /* A positive nCol means the columns names for this view
+ * are already known.
*/
- if (pTable->nCol > 0)
- return 0;
+ if (table->nCol > 0)
+ return true;
- /* 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:
+ /* 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:
+ * 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 (pTable->nCol < 0) {
- sqlite3ErrorMsg(pParse, "view %s is circularly defined",
- pTable->zName);
- return 1;
- }
- assert(pTable->nCol >= 0);
-
- /* 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.
+ if (table->nCol < 0) {
+ sqlite3ErrorMsg(parse, "view %s is circularly defined",
+ table->zName);
+ return false;
+ }
+ assert(table->nCol >= 0);
+
+ /* 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(pTable->pSelect);
- pSel = sqlite3SelectDup(db, pTable->pSelect, 0);
- if (pSel) {
- n = pParse->nTab;
- sqlite3SrcListAssignCursors(pParse, pSel->pSrc);
- pTable->nCol = -1;
- db->lookaside.bDisable++;
- pSelTab = sqlite3ResultSetOfSelect(pParse, pSel);
- pParse->nTab = n;
- if (pTable->pCheck) {
- /* CREATE VIEW name(arglist) AS ...
- * The names of the columns in the table are taken from
- * arglist which is stored in pTable->pCheck. The pCheck field
- * normally holds CHECK constraints on an ordinary table, but for
- * a VIEW it holds the list of column names.
- */
- sqlite3ColumnsFromExprList(pParse, pTable->pCheck,
- &pTable->nCol,
- &pTable->aCol);
- if (db->mallocFailed == 0 && pParse->nErr == 0
- && pTable->nCol == pSel->pEList->nExpr) {
- sqlite3SelectAddColumnTypeAndCollation(pParse,
- pTable,
- pSel);
- }
- } else if (pSelTab) {
- /* CREATE VIEW name AS... without an argument list. Construct
- * the column names from the SELECT statement that defines the view.
- */
- assert(pTable->aCol == 0);
- pTable->nCol = pSelTab->nCol;
- pTable->aCol = pSelTab->aCol;
- pSelTab->nCol = 0;
- pSelTab->aCol = 0;
- } else {
- pTable->nCol = 0;
- nErr++;
+ assert(table->pSelect != NULL);
+ sqlite3 *db = parse->db;
+ struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
+ if (select == NULL)
+ return false;
+ int n = parse->nTab;
+ sqlite3SrcListAssignCursors(parse, select->pSrc);
+ table->nCol = -1;
+ db->lookaside.bDisable++;
+ struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
+ parse->nTab = n;
+ bool is_ok = true;
+ if (table->pCheck != 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.
+ */
+ sqlite3ColumnsFromExprList(parse, table->pCheck,
+ &table->nCol,
+ &table->aCol);
+ if (db->mallocFailed == 0 && parse->nErr == 0
+ && table->nCol == select->pEList->nExpr) {
+ sqlite3SelectAddColumnTypeAndCollation(parse, table,
+ select);
}
- sqlite3DeleteTable(db, pSelTab);
- sqlite3SelectDelete(db, pSel);
- db->lookaside.bDisable--;
+ } 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);
+ table->nCol = sel_tab->nCol;
+ table->aCol = sel_tab->aCol;
+ sel_tab->nCol = 0;
+ sel_tab->aCol = NULL;
} else {
- nErr++;
+ table->nCol = 0;
+ is_ok = false;
}
-#endif /* SQLITE_OMIT_VIEW */
- return nErr;
+ sqlite3DeleteTable(db, sel_tab);
+ sqlite3SelectDelete(db, select);
+ db->lookaside.bDisable--;
+
+ return is_ok;
}
-#endif /* !defined(SQLITE_OMIT_VIEW) */
#ifndef SQLITE_OMIT_VIEW
/*
@@ -2672,7 +2662,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
VdbeCoverage(v);
sqlite3VdbeJumpHere(v, addr1);
if (memRootPage < 0)
- sqlite3VdbeAddOp2(v, OP_Clear, tnum, 0);
+ sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
+ 0);
emit_open_cursor(pParse, iIdx, tnum);
sqlite3VdbeChangeP5(v,
OPFLAG_BULKCSR | ((memRootPage >= 0) ?
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f4d248e..e073f0c 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
+#include "box/box.h"
#include "box/session.h"
#include "box/schema.h"
#include "sqliteInt.h"
@@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* with multiple tables and expect an SrcList* parameter
* instead of just a Table* parameter.
*/
- struct Table *table = sql_list_lookup_table(parse, tab_list);
- if (table == NULL)
- goto delete_from_cleanup;
-
- struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(table->tnum));
- assert(space != NULL);
+ struct Table *table = NULL;
+ struct space *space;
+ uint32_t space_id;
/* Figure out if we have any triggers and if the table
* being deleted from is a view.
*/
- struct Trigger *trigger_list = sqlite3TriggersExist(table, TK_DELETE,
- NULL, NULL);
-
- bool is_view = space->def->opts.is_view;
+ struct Trigger *trigger_list = NULL;
/* True if there are triggers or FKs or subqueries in the
* WHERE clause.
*/
- bool is_complex = trigger_list != NULL ||
- sqlite3FkRequired(table, NULL);
+ bool is_complex = false;
+ const char *tab_name = tab_list->a->zName;
+ if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
+ space_id = box_space_id_by_name(tab_name,
+ strlen(tab_name));
+ if (space_id == BOX_ID_NIL)
+ goto delete_from_cleanup;
+ } else {
+ table = sql_list_lookup_table(parse, tab_list);
+ if (table == NULL)
+ goto delete_from_cleanup;
+ space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+ trigger_list =sqlite3TriggersExist(table, TK_DELETE,
+ NULL, NULL);
+ is_complex = trigger_list != NULL ||
+ sqlite3FkRequired(table, NULL);
+ }
+ space = space_by_id(space_id);
+ assert(space != NULL);
+
+ bool is_view = space->def->opts.is_view;
/* If table is really a view, make sure it has been
* initialized.
*/
- if (sqlite3ViewGetColumnNames(parse, table))
- goto delete_from_cleanup;
+ if (is_view) {
+ if (!sql_view_column_names(parse, table))
+ goto delete_from_cleanup;
- if (is_view && trigger_list == NULL) {
- sqlite3ErrorMsg(parse, "cannot modify %s because it is a view",
- space->def->name);
- goto delete_from_cleanup;
+ if (trigger_list == NULL) {
+ sqlite3ErrorMsg(parse, "cannot modify %s because it is a"
+ " view", space->def->name);
+ goto delete_from_cleanup;
+ }
}
/* Assign cursor numbers to the table and all its indices.
@@ -139,14 +155,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
tab_cursor);
}
- /* Resolve the column names in the WHERE clause. */
- struct NameContext nc;
- memset(&nc, 0, sizeof(nc));
- nc.pParse = parse;
- nc.pSrcList = tab_list;
- if (sqlite3ResolveExprNames(&nc, where))
- goto delete_from_cleanup;
-
/* Initialize the counter of the number of rows deleted,
* if we are counting rows.
*/
@@ -162,7 +170,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (where == NULL && !is_complex) {
assert(!is_view);
- sqlite3VdbeAddOp1(v, OP_Clear, table->tnum);
+ sqlite3VdbeAddOp1(v, OP_Clear, space_id);
/* Do not start Tarantool's transaction in case of
* truncate optimization. This is workaround until
@@ -171,6 +179,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
*/
parse->initiateTTrans = false;
} else {
+ /* Resolve the column names in the WHERE clause. */
+ struct NameContext nc;
+ memset(&nc, 0, sizeof(nc));
+ nc.pParse = parse;
+ nc.pSrcList = tab_list;
+ if (sqlite3ResolveExprNames(&nc, where))
+ goto delete_from_cleanup;
uint16_t wcf = WHERE_ONEPASS_DESIRED | WHERE_DUPLICATES_OK |
WHERE_SEEK_TABLE;
if (nc.ncFlags & NC_VarSelect)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index e23cffd..9e37def 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -349,11 +349,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
int *aRegIdx = 0; /* One register allocated to each index */
uint32_t space_id = 0;
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True if attempting to insert into a view */
+ bool is_view; /* True if attempting to insert into a view */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of trigger times */
-#endif
db = pParse->db;
memset(&dest, 0, sizeof(dest));
@@ -389,28 +387,18 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Figure out if we have any triggers and if the table being
* inserted into is a view
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_INSERT, 0, &tmask);
- isView = space_is_view(pTab);
-#else
-#define pTrigger 0
-#define tmask 0
-#define isView 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
+ is_view = space_is_view(pTab);
assert((pTrigger && tmask) || (pTrigger == 0 && tmask == 0));
/* If pTab is really a view, make sure it has been initialized.
* ViewGetColumnNames() is a no-op if pTab is not a view.
*/
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (is_view && !sql_view_column_names(pParse, pTab))
goto insert_cleanup;
/* Cannot insert into a read-only table. */
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto insert_cleanup;
@@ -481,7 +469,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
bIdListInOrder = 0;
if (j == pTab->iPKey) {
ipkColumn = i;
- assert(isView);
+ assert(is_view);
}
break;
}
@@ -637,7 +625,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
}
/* If this is not a view, open the table and and all indices */
- if (!isView) {
+ if (!is_view) {
int nIdx;
nIdx =
sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0,
@@ -731,7 +719,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
* If this is a real table, attempt conversions as required by the
* table column affinities.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3TableAffinity(v, pTab, regCols + 1);
}
@@ -747,7 +735,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Compute the content of the next row to insert into a range of
* registers beginning at regIns.
*/
- if (!isView) {
+ if (!is_view) {
if (ipkColumn >= 0) {
if (useTempTable) {
sqlite3VdbeAddOp3(v, OP_Column, srcTab,
@@ -930,20 +918,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */
sqlite3DbFree(db, aRegIdx);
}
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-#ifdef tmask
-#undef tmask
-#endif
-
/*
* Meanings of bits in of pWalker->eCode for checkConstraintUnchanged()
*/
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index e4ceec2..634da87 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -357,7 +357,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Column *pCol;
Index *pPk = sqlite3PrimaryKeyIndex(pTab);
pParse->nMem = 6;
- sqlite3ViewGetColumnNames(pParse, pTab);
+ if (space_is_view(pTab))
+ sql_view_column_names(pParse, pTab);
for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
i++, pCol++) {
if (!table_column_is_in_pk(pTab, i)) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index de111d3..3ae40f1 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4680,7 +4680,7 @@ selectExpander(Walker * pWalker, Select * p)
#if !defined(SQLITE_OMIT_VIEW)
if (space_is_view(pTab)) {
i16 nCol;
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (!sql_view_column_names(pParse, pTab))
return WRC_Abort;
assert(pFrom->pSelect == 0);
pFrom->pSelect =
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ab50c3e..1c17350 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3592,11 +3592,18 @@ int sqlite3FaultSim(int);
void sqlite3CreateView(Parse *, Token *, Token *, ExprList *, Select *, int);
-#if !defined(SQLITE_OMIT_VIEW)
-int sqlite3ViewGetColumnNames(Parse *, Table *);
-#else
-#define sqlite3ViewGetColumnNames(A,B) 0
-#endif
+/**
+ * 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.
+ *
+ * @param parse Parsing context.
+ * @param table Tables to process.
+ * @retval True if success.
+ */
+bool
+sql_view_column_names(struct Parse *parse, struct Table *table);
void
sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index c521a3b..ec240d3 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -137,11 +137,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
int labelContinue; /* Jump here to continue next step of UPDATE loop */
struct session *user_session = current_session();
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True when updating a view (INSTEAD OF trigger) */
+ bool is_view; /* True when updating a view (INSTEAD OF trigger) */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
-#endif
int newmask; /* Mask of NEW.* columns accessed by BEFORE triggers */
int iEph = 0; /* Ephemeral table holding all primary key values */
int nKey = 0; /* Number of elements in regKey */
@@ -170,24 +168,14 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* Figure out if we have any triggers and if the table being
* updated is a view.
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_UPDATE, pChanges, &tmask);
- isView = space_is_view(pTab);
+ is_view = space_is_view(pTab);
assert(pTrigger || tmask == 0);
-#else
-#define pTrigger 0
-#define isView 0
-#define tmask 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
- if (sqlite3ViewGetColumnNames(pParse, pTab)) {
+ if (is_view && !sql_view_column_names(pParse, pTab)) {
goto update_cleanup;
}
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto update_cleanup;
@@ -200,7 +188,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
*/
pTabList->a[0].iCursor = iBaseCur = iDataCur = pParse->nTab++;
iIdxCur = iDataCur + 1;
- pPk = isView ? 0 : sqlite3PrimaryKeyIndex(pTab);
+ pPk = is_view ? 0 : sqlite3PrimaryKeyIndex(pTab);
for (nIdx = 0, pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext, nIdx++) {
if (IsPrimaryKeyIndex(pIdx) && pPk != 0) {
iDataCur = pParse->nTab;
@@ -324,13 +312,11 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* If we are trying to update a view, realize that view into
* an ephemeral table.
*/
-#if !defined(SQLITE_OMIT_VIEW) && !defined(SQLITE_OMIT_TRIGGER)
- if (isView) {
+ if (is_view) {
sql_materialize_view(pParse, pTab->zName, pWhere, iDataCur);
/* Number of columns from SELECT plus ID.*/
nKey = pTab->nCol + 1;
}
-#endif
/* Resolve the column names in all the expressions in the
* WHERE clause.
@@ -347,7 +333,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
i16 nPk; /* Number of components of the PRIMARY KEY */
int addrOpen; /* Address of the OpenEphemeral instruction */
- if (isView) {
+ if (is_view) {
nPk = nKey;
} else {
assert(pPk != 0);
@@ -359,7 +345,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
iEph = pParse->nTab++;
sqlite3VdbeAddOp2(v, OP_Null, 0, iPk);
- if (isView) {
+ if (is_view) {
addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph,
nKey);
} else {
@@ -372,7 +358,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
if (pWInfo == 0)
goto update_cleanup;
okOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass);
- if (isView){
+ if (is_view) {
for (i = 0; i < nPk; i++) {
sqlite3VdbeAddOp3(v, OP_Column, iDataCur, i, iPk + i);
}
@@ -390,7 +376,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
nKey = nPk;
regKey = iPk;
} else {
- const char *zAff = isView ? 0 :
+ const char *zAff = is_view ? 0 :
sqlite3IndexAffinityStr(pParse->db, pPk);
sqlite3VdbeAddOp4(v, OP_MakeRecord, iPk, nPk, regKey,
zAff, nPk);
@@ -412,7 +398,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
labelBreak = sqlite3VdbeMakeLabel(v);
- if (!isView) {
+ if (!is_view) {
/*
* Open every index that needs updating. Note that if any
* index could potentially invoke a REPLACE conflict resolution
@@ -445,7 +431,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
labelContinue = labelBreak;
sqlite3VdbeAddOp2(v, OP_IsNull, regKey,
labelBreak);
- if (aToOpen[iDataCur - iBaseCur] && !isView) {
+ if (aToOpen[iDataCur - iBaseCur] && !is_view) {
assert(pPk);
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelBreak, regKey, nKey);
@@ -552,7 +538,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
* is deleted or renamed by a BEFORE trigger - is left undefined in the
* documentation.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelContinue, regKey, nKey);
VdbeCoverage(v);
@@ -576,7 +562,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
}
- if (!isView) {
+ if (!is_view) {
int addr1 = 0; /* Address of jump instruction */
int bReplace = 0; /* True if REPLACE conflict resolution might happen */
@@ -691,15 +677,3 @@ sqlite3Update(Parse * pParse, /* The parser context */
sql_expr_free(db, pWhere, false);
return;
}
-
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 455198d..9acb9b6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4614,7 +4614,7 @@ case OP_IdxGE: { /* jump */
*/
case OP_Clear: {
assert(pOp->p1 > 0);
- uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pOp->p1);
+ uint32_t space_id = pOp->p1;
struct space *space = space_by_id(space_id);
assert(space != NULL);
rc = tarantoolSqlite3ClearTable(space);
diff --git a/test/sql-tap/delete1.test.lua b/test/sql-tap/delete1.test.lua
index 61d7756..810ca8a 100755
--- a/test/sql-tap/delete1.test.lua
+++ b/test/sql-tap/delete1.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(7)
+test:plan(9)
--!./tcltestrunner.lua
-- ["set","testdir",[["file","dirname",["argv0"]]]]
@@ -131,5 +131,26 @@ test:do_test(
-- </delete1-5.0>
})
+-- Tests for data dictionary integration.
+s = box.schema.create_space('t')
+i = s:create_index('i', {parts={1, 'unsigned'}})
+test:do_test(
+ "delete1-6.0",
+ function()
+ s:replace({1})
+ s:replace({2})
+ s:replace({3})
+ return s:count()
+ end,
+ 3)
+
+test:do_test(
+ "delete1-6.1",
+ function()
+ box.sql.execute([[delete from "t"]])
+ return s:count()
+ end,
+ 0)
+
test:finish_test()
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-21 8:20 ` Kirill Yukhin
@ 2018-05-21 9:47 ` Vladislav Shpilevoy
2018-05-21 10:25 ` Kirill Yukhin
0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-21 9:47 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: tarantool-patches
Hello. Thanks for the fixes!
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index df75186..68f81e8 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
> }
> #endif /* SQLITE_OMIT_VIEW */
>
> -#if !defined(SQLITE_OMIT_VIEW)
> -/*
> - * The Table structure pTable is really a VIEW. Fill in the names of
> - * the columns of the view in the pTable structure. Return the number
> - * of errors. If an error is seen leave an error message in pParse->zErrMsg.
> - */
> -int
> -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
> +bool
In Tarantool code style we do not return boolean on non-check functions.
0 or -1, please.
The rest of patch is ok.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-21 9:47 ` Vladislav Shpilevoy
@ 2018-05-21 10:25 ` Kirill Yukhin
2018-05-21 10:49 ` Vladislav Shpilevoy
0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2018-05-21 10:25 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 21 мая 12:47, Vladislav Shpilevoy wrote:
> Hello. Thanks for the fixes!
>
> > diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> > index df75186..68f81e8 100644
> > --- a/src/box/sql/build.c
> > +++ b/src/box/sql/build.c
> > @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
> > -int
> > -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
> > +bool
>
> In Tarantool code style we do not return boolean on non-check functions.
> 0 or -1, please.
Done.
--
Regards, Kirill Yukhin
commit ab5fabef4625312bfe5534c28487aca139d3d658
Author: Kirill Yukhin <kyukhin@tarantool.org>
Date: Fri May 18 16:31:24 2018 +0300
sql: refactor SQL delete to allow Lua spaces
This is a first step toward fully-featured deletion
of spaces created in Lua by means of SQL language.
This change to handle most simple case:
DELETE * FROM <space_name> and will be improved in
nearest future.
Also, refactor (sqlite3ViewGetColumnNames()) to
use Tarantool's code style and naming convention.
Part of #3235
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 998dd5b..cc594a7 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -169,7 +169,8 @@ openStatTable(Parse * pParse, /* Parsing context */
* The sql_stat[134] table already exists.
* Delete all rows.
*/
- sqlite3VdbeAddOp2(v, OP_Clear, aRoot[i], 0);
+ sqlite3VdbeAddOp2(v, OP_Clear,
+ SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
}
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index df75186..f59087b 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
}
#endif /* SQLITE_OMIT_VIEW */
-#if !defined(SQLITE_OMIT_VIEW)
-/*
- * The Table structure pTable is really a VIEW. Fill in the names of
- * the columns of the view in the pTable structure. Return the number
- * of errors. If an error is seen leave an error message in pParse->zErrMsg.
- */
int
-sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
+sql_view_column_names(struct Parse *parse, struct Table *table)
{
- Table *pSelTab; /* A fake table from which we get the result set */
- Select *pSel; /* Copy of the SELECT that implements the view */
- int nErr = 0; /* Number of errors encountered */
- int n; /* Temporarily holds the number of cursors assigned */
- sqlite3 *db = pParse->db; /* Database connection for malloc errors */
-
- assert(pTable);
-
-#ifndef SQLITE_OMIT_VIEW
- /* A positive nCol means the columns names for this view are
- * already known.
+ assert(table != NULL);
+ assert(space_is_view(table));
+ /* A positive nCol means the columns names for this view
+ * are already known.
*/
- if (pTable->nCol > 0)
- return 0;
+ if (table->nCol > 0)
+ return true;
- /* 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:
+ /* 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:
+ * 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 (pTable->nCol < 0) {
- sqlite3ErrorMsg(pParse, "view %s is circularly defined",
- pTable->zName);
- return 1;
- }
- assert(pTable->nCol >= 0);
-
- /* 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.
+ if (table->nCol < 0) {
+ sqlite3ErrorMsg(parse, "view %s is circularly defined",
+ table->zName);
+ return false;
+ }
+ assert(table->nCol >= 0);
+
+ /* 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(pTable->pSelect);
- pSel = sqlite3SelectDup(db, pTable->pSelect, 0);
- if (pSel) {
- n = pParse->nTab;
- sqlite3SrcListAssignCursors(pParse, pSel->pSrc);
- pTable->nCol = -1;
- db->lookaside.bDisable++;
- pSelTab = sqlite3ResultSetOfSelect(pParse, pSel);
- pParse->nTab = n;
- if (pTable->pCheck) {
- /* CREATE VIEW name(arglist) AS ...
- * The names of the columns in the table are taken from
- * arglist which is stored in pTable->pCheck. The pCheck field
- * normally holds CHECK constraints on an ordinary table, but for
- * a VIEW it holds the list of column names.
- */
- sqlite3ColumnsFromExprList(pParse, pTable->pCheck,
- &pTable->nCol,
- &pTable->aCol);
- if (db->mallocFailed == 0 && pParse->nErr == 0
- && pTable->nCol == pSel->pEList->nExpr) {
- sqlite3SelectAddColumnTypeAndCollation(pParse,
- pTable,
- pSel);
- }
- } else if (pSelTab) {
- /* CREATE VIEW name AS... without an argument list. Construct
- * the column names from the SELECT statement that defines the view.
- */
- assert(pTable->aCol == 0);
- pTable->nCol = pSelTab->nCol;
- pTable->aCol = pSelTab->aCol;
- pSelTab->nCol = 0;
- pSelTab->aCol = 0;
- } else {
- pTable->nCol = 0;
- nErr++;
+ assert(table->pSelect != NULL);
+ sqlite3 *db = parse->db;
+ struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
+ if (select == NULL)
+ return false;
+ int n = parse->nTab;
+ sqlite3SrcListAssignCursors(parse, select->pSrc);
+ table->nCol = -1;
+ db->lookaside.bDisable++;
+ struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
+ parse->nTab = n;
+ bool is_ok = true;
+ if (table->pCheck != 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.
+ */
+ sqlite3ColumnsFromExprList(parse, table->pCheck,
+ &table->nCol,
+ &table->aCol);
+ if (db->mallocFailed == 0 && parse->nErr == 0
+ && table->nCol == select->pEList->nExpr) {
+ sqlite3SelectAddColumnTypeAndCollation(parse, table,
+ select);
}
- sqlite3DeleteTable(db, pSelTab);
- sqlite3SelectDelete(db, pSel);
- db->lookaside.bDisable--;
+ } 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);
+ table->nCol = sel_tab->nCol;
+ table->aCol = sel_tab->aCol;
+ sel_tab->nCol = 0;
+ sel_tab->aCol = NULL;
} else {
- nErr++;
+ table->nCol = 0;
+ is_ok = false;
}
-#endif /* SQLITE_OMIT_VIEW */
- return nErr;
+ sqlite3DeleteTable(db, sel_tab);
+ sqlite3SelectDelete(db, select);
+ db->lookaside.bDisable--;
+
+ return is_ok;
}
-#endif /* !defined(SQLITE_OMIT_VIEW) */
#ifndef SQLITE_OMIT_VIEW
/*
@@ -2672,7 +2662,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
VdbeCoverage(v);
sqlite3VdbeJumpHere(v, addr1);
if (memRootPage < 0)
- sqlite3VdbeAddOp2(v, OP_Clear, tnum, 0);
+ sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
+ 0);
emit_open_cursor(pParse, iIdx, tnum);
sqlite3VdbeChangeP5(v,
OPFLAG_BULKCSR | ((memRootPage >= 0) ?
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f4d248e..8c7cd7d 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
+#include "box/box.h"
#include "box/session.h"
#include "box/schema.h"
#include "sqliteInt.h"
@@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* with multiple tables and expect an SrcList* parameter
* instead of just a Table* parameter.
*/
- struct Table *table = sql_list_lookup_table(parse, tab_list);
- if (table == NULL)
- goto delete_from_cleanup;
-
- struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(table->tnum));
- assert(space != NULL);
+ struct Table *table = NULL;
+ struct space *space;
+ uint32_t space_id;
/* Figure out if we have any triggers and if the table
* being deleted from is a view.
*/
- struct Trigger *trigger_list = sqlite3TriggersExist(table, TK_DELETE,
- NULL, NULL);
-
- bool is_view = space->def->opts.is_view;
+ struct Trigger *trigger_list = NULL;
/* True if there are triggers or FKs or subqueries in the
* WHERE clause.
*/
- bool is_complex = trigger_list != NULL ||
- sqlite3FkRequired(table, NULL);
+ bool is_complex = false;
+ const char *tab_name = tab_list->a->zName;
+ if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
+ space_id = box_space_id_by_name(tab_name,
+ strlen(tab_name));
+ if (space_id == BOX_ID_NIL)
+ goto delete_from_cleanup;
+ } else {
+ table = sql_list_lookup_table(parse, tab_list);
+ if (table == NULL)
+ goto delete_from_cleanup;
+ space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+ trigger_list =sqlite3TriggersExist(table, TK_DELETE,
+ NULL, NULL);
+ is_complex = trigger_list != NULL ||
+ sqlite3FkRequired(table, NULL);
+ }
+ space = space_by_id(space_id);
+ assert(space != NULL);
+
+ bool is_view = space->def->opts.is_view;
/* If table is really a view, make sure it has been
* initialized.
*/
- if (sqlite3ViewGetColumnNames(parse, table))
- goto delete_from_cleanup;
+ if (is_view) {
+ if (sql_view_column_names(parse, table) != 0)
+ goto delete_from_cleanup;
- if (is_view && trigger_list == NULL) {
- sqlite3ErrorMsg(parse, "cannot modify %s because it is a view",
- space->def->name);
- goto delete_from_cleanup;
+ if (trigger_list == NULL) {
+ sqlite3ErrorMsg(parse, "cannot modify %s because it is a"
+ " view", space->def->name);
+ goto delete_from_cleanup;
+ }
}
/* Assign cursor numbers to the table and all its indices.
@@ -139,14 +155,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
tab_cursor);
}
- /* Resolve the column names in the WHERE clause. */
- struct NameContext nc;
- memset(&nc, 0, sizeof(nc));
- nc.pParse = parse;
- nc.pSrcList = tab_list;
- if (sqlite3ResolveExprNames(&nc, where))
- goto delete_from_cleanup;
-
/* Initialize the counter of the number of rows deleted,
* if we are counting rows.
*/
@@ -162,7 +170,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (where == NULL && !is_complex) {
assert(!is_view);
- sqlite3VdbeAddOp1(v, OP_Clear, table->tnum);
+ sqlite3VdbeAddOp1(v, OP_Clear, space_id);
/* Do not start Tarantool's transaction in case of
* truncate optimization. This is workaround until
@@ -171,6 +179,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
*/
parse->initiateTTrans = false;
} else {
+ /* Resolve the column names in the WHERE clause. */
+ struct NameContext nc;
+ memset(&nc, 0, sizeof(nc));
+ nc.pParse = parse;
+ nc.pSrcList = tab_list;
+ if (sqlite3ResolveExprNames(&nc, where))
+ goto delete_from_cleanup;
uint16_t wcf = WHERE_ONEPASS_DESIRED | WHERE_DUPLICATES_OK |
WHERE_SEEK_TABLE;
if (nc.ncFlags & NC_VarSelect)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index e23cffd..5d62e58 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -349,11 +349,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
int *aRegIdx = 0; /* One register allocated to each index */
uint32_t space_id = 0;
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True if attempting to insert into a view */
+ bool is_view; /* True if attempting to insert into a view */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of trigger times */
-#endif
db = pParse->db;
memset(&dest, 0, sizeof(dest));
@@ -389,28 +387,18 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Figure out if we have any triggers and if the table being
* inserted into is a view
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_INSERT, 0, &tmask);
- isView = space_is_view(pTab);
-#else
-#define pTrigger 0
-#define tmask 0
-#define isView 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
+ is_view = space_is_view(pTab);
assert((pTrigger && tmask) || (pTrigger == 0 && tmask == 0));
/* If pTab is really a view, make sure it has been initialized.
* ViewGetColumnNames() is a no-op if pTab is not a view.
*/
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (is_view && sql_view_column_names(pParse, pTab) != 0)
goto insert_cleanup;
/* Cannot insert into a read-only table. */
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto insert_cleanup;
@@ -481,7 +469,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
bIdListInOrder = 0;
if (j == pTab->iPKey) {
ipkColumn = i;
- assert(isView);
+ assert(is_view);
}
break;
}
@@ -637,7 +625,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
}
/* If this is not a view, open the table and and all indices */
- if (!isView) {
+ if (!is_view) {
int nIdx;
nIdx =
sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0,
@@ -731,7 +719,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
* If this is a real table, attempt conversions as required by the
* table column affinities.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3TableAffinity(v, pTab, regCols + 1);
}
@@ -747,7 +735,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Compute the content of the next row to insert into a range of
* registers beginning at regIns.
*/
- if (!isView) {
+ if (!is_view) {
if (ipkColumn >= 0) {
if (useTempTable) {
sqlite3VdbeAddOp3(v, OP_Column, srcTab,
@@ -930,20 +918,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */
sqlite3DbFree(db, aRegIdx);
}
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-#ifdef tmask
-#undef tmask
-#endif
-
/*
* Meanings of bits in of pWalker->eCode for checkConstraintUnchanged()
*/
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index e4ceec2..634da87 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -357,7 +357,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Column *pCol;
Index *pPk = sqlite3PrimaryKeyIndex(pTab);
pParse->nMem = 6;
- sqlite3ViewGetColumnNames(pParse, pTab);
+ if (space_is_view(pTab))
+ sql_view_column_names(pParse, pTab);
for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
i++, pCol++) {
if (!table_column_is_in_pk(pTab, i)) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index de111d3..e4d4013 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4680,7 +4680,7 @@ selectExpander(Walker * pWalker, Select * p)
#if !defined(SQLITE_OMIT_VIEW)
if (space_is_view(pTab)) {
i16 nCol;
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (sql_view_column_names(pParse, pTab) != 0)
return WRC_Abort;
assert(pFrom->pSelect == 0);
pFrom->pSelect =
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ab50c3e..26db466 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3592,11 +3592,18 @@ int sqlite3FaultSim(int);
void sqlite3CreateView(Parse *, Token *, Token *, ExprList *, Select *, int);
-#if !defined(SQLITE_OMIT_VIEW)
-int sqlite3ViewGetColumnNames(Parse *, Table *);
-#else
-#define sqlite3ViewGetColumnNames(A,B) 0
-#endif
+/**
+ * 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.
+ *
+ * @param parse Parsing context.
+ * @param table Tables to process.
+ * @retval 0 if success, -1 in case of error.
+ */
+int
+sql_view_column_names(struct Parse *parse, struct Table *table);
void
sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index c521a3b..4d6d0e1 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -137,11 +137,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
int labelContinue; /* Jump here to continue next step of UPDATE loop */
struct session *user_session = current_session();
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True when updating a view (INSTEAD OF trigger) */
+ bool is_view; /* True when updating a view (INSTEAD OF trigger) */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
-#endif
int newmask; /* Mask of NEW.* columns accessed by BEFORE triggers */
int iEph = 0; /* Ephemeral table holding all primary key values */
int nKey = 0; /* Number of elements in regKey */
@@ -170,24 +168,14 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* Figure out if we have any triggers and if the table being
* updated is a view.
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_UPDATE, pChanges, &tmask);
- isView = space_is_view(pTab);
+ is_view = space_is_view(pTab);
assert(pTrigger || tmask == 0);
-#else
-#define pTrigger 0
-#define isView 0
-#define tmask 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
- if (sqlite3ViewGetColumnNames(pParse, pTab)) {
+ if (is_view && sql_view_column_names(pParse, pTab) != 0) {
goto update_cleanup;
}
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto update_cleanup;
@@ -200,7 +188,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
*/
pTabList->a[0].iCursor = iBaseCur = iDataCur = pParse->nTab++;
iIdxCur = iDataCur + 1;
- pPk = isView ? 0 : sqlite3PrimaryKeyIndex(pTab);
+ pPk = is_view ? 0 : sqlite3PrimaryKeyIndex(pTab);
for (nIdx = 0, pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext, nIdx++) {
if (IsPrimaryKeyIndex(pIdx) && pPk != 0) {
iDataCur = pParse->nTab;
@@ -324,13 +312,11 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* If we are trying to update a view, realize that view into
* an ephemeral table.
*/
-#if !defined(SQLITE_OMIT_VIEW) && !defined(SQLITE_OMIT_TRIGGER)
- if (isView) {
+ if (is_view) {
sql_materialize_view(pParse, pTab->zName, pWhere, iDataCur);
/* Number of columns from SELECT plus ID.*/
nKey = pTab->nCol + 1;
}
-#endif
/* Resolve the column names in all the expressions in the
* WHERE clause.
@@ -347,7 +333,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
i16 nPk; /* Number of components of the PRIMARY KEY */
int addrOpen; /* Address of the OpenEphemeral instruction */
- if (isView) {
+ if (is_view) {
nPk = nKey;
} else {
assert(pPk != 0);
@@ -359,7 +345,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
iEph = pParse->nTab++;
sqlite3VdbeAddOp2(v, OP_Null, 0, iPk);
- if (isView) {
+ if (is_view) {
addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph,
nKey);
} else {
@@ -372,7 +358,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
if (pWInfo == 0)
goto update_cleanup;
okOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass);
- if (isView){
+ if (is_view) {
for (i = 0; i < nPk; i++) {
sqlite3VdbeAddOp3(v, OP_Column, iDataCur, i, iPk + i);
}
@@ -390,7 +376,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
nKey = nPk;
regKey = iPk;
} else {
- const char *zAff = isView ? 0 :
+ const char *zAff = is_view ? 0 :
sqlite3IndexAffinityStr(pParse->db, pPk);
sqlite3VdbeAddOp4(v, OP_MakeRecord, iPk, nPk, regKey,
zAff, nPk);
@@ -412,7 +398,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
labelBreak = sqlite3VdbeMakeLabel(v);
- if (!isView) {
+ if (!is_view) {
/*
* Open every index that needs updating. Note that if any
* index could potentially invoke a REPLACE conflict resolution
@@ -445,7 +431,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
labelContinue = labelBreak;
sqlite3VdbeAddOp2(v, OP_IsNull, regKey,
labelBreak);
- if (aToOpen[iDataCur - iBaseCur] && !isView) {
+ if (aToOpen[iDataCur - iBaseCur] && !is_view) {
assert(pPk);
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelBreak, regKey, nKey);
@@ -552,7 +538,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
* is deleted or renamed by a BEFORE trigger - is left undefined in the
* documentation.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelContinue, regKey, nKey);
VdbeCoverage(v);
@@ -576,7 +562,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
}
- if (!isView) {
+ if (!is_view) {
int addr1 = 0; /* Address of jump instruction */
int bReplace = 0; /* True if REPLACE conflict resolution might happen */
@@ -691,15 +677,3 @@ sqlite3Update(Parse * pParse, /* The parser context */
sql_expr_free(db, pWhere, false);
return;
}
-
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 455198d..9acb9b6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4614,7 +4614,7 @@ case OP_IdxGE: { /* jump */
*/
case OP_Clear: {
assert(pOp->p1 > 0);
- uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pOp->p1);
+ uint32_t space_id = pOp->p1;
struct space *space = space_by_id(space_id);
assert(space != NULL);
rc = tarantoolSqlite3ClearTable(space);
diff --git a/test/sql-tap/delete1.test.lua b/test/sql-tap/delete1.test.lua
index 61d7756..810ca8a 100755
--- a/test/sql-tap/delete1.test.lua
+++ b/test/sql-tap/delete1.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(7)
+test:plan(9)
--!./tcltestrunner.lua
-- ["set","testdir",[["file","dirname",["argv0"]]]]
@@ -131,5 +131,26 @@ test:do_test(
-- </delete1-5.0>
})
+-- Tests for data dictionary integration.
+s = box.schema.create_space('t')
+i = s:create_index('i', {parts={1, 'unsigned'}})
+test:do_test(
+ "delete1-6.0",
+ function()
+ s:replace({1})
+ s:replace({2})
+ s:replace({3})
+ return s:count()
+ end,
+ 3)
+
+test:do_test(
+ "delete1-6.1",
+ function()
+ box.sql.execute([[delete from "t"]])
+ return s:count()
+ end,
+ 0)
+
test:finish_test()
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-21 10:25 ` Kirill Yukhin
@ 2018-05-21 10:49 ` Vladislav Shpilevoy
2018-05-21 11:24 ` Kirill Yukhin
0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-21 10:49 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: tarantool-patches
On 21/05/2018 13:25, Kirill Yukhin wrote:
> On 21 мая 12:47, Vladislav Shpilevoy wrote:
>> Hello. Thanks for the fixes!
>>
>>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>>> index df75186..68f81e8 100644
>>> --- a/src/box/sql/build.c
>>> +++ b/src/box/sql/build.c
>>> @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
>>> -int
>>> -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
>>> +bool
>>
>> In Tarantool code style we do not return boolean on non-check functions.
>> 0 or -1, please.
> Done.
No(( Tests are failing. See 5 comments below, why.
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index df75186..f59087b 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> int
> -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
> +sql_view_column_names(struct Parse *parse, struct Table *table)
> {
> - Table *pSelTab; /* A fake table from which we get the result set */
> - Select *pSel; /* Copy of the SELECT that implements the view */
> - int nErr = 0; /* Number of errors encountered */
> - int n; /* Temporarily holds the number of cursors assigned */
> - sqlite3 *db = pParse->db; /* Database connection for malloc errors */
> -
> - assert(pTable);
> -
> -#ifndef SQLITE_OMIT_VIEW
> - /* A positive nCol means the columns names for this view are
> - * already known.
> + assert(table != NULL);
> + assert(space_is_view(table));
> + /* A positive nCol means the columns names for this view
> + * are already known.
> */
> - if (pTable->nCol > 0)
> - return 0;
> + if (table->nCol > 0)
> + return true;
1. Here you return 'true', but returned type is int. It is converted to 1,
and this function is treated as failed.
>
> - /* 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:
> + /* 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:
> + * 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 (pTable->nCol < 0) {
> - sqlite3ErrorMsg(pParse, "view %s is circularly defined",
> - pTable->zName);
> - return 1;
> - }
> - assert(pTable->nCol >= 0);
> -
> - /* 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.
> + if (table->nCol < 0) {
> + sqlite3ErrorMsg(parse, "view %s is circularly defined",
> + table->zName);
> + return false;
2. Same.
> + }
> + assert(table->nCol >= 0);
> +
> + /* 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(pTable->pSelect);
> - pSel = sqlite3SelectDup(db, pTable->pSelect, 0);
> - if (pSel) {
> - n = pParse->nTab;
> - sqlite3SrcListAssignCursors(pParse, pSel->pSrc);
> - pTable->nCol = -1;
> - db->lookaside.bDisable++;
> - pSelTab = sqlite3ResultSetOfSelect(pParse, pSel);
> - pParse->nTab = n;
> - if (pTable->pCheck) {
> - /* CREATE VIEW name(arglist) AS ...
> - * The names of the columns in the table are taken from
> - * arglist which is stored in pTable->pCheck. The pCheck field
> - * normally holds CHECK constraints on an ordinary table, but for
> - * a VIEW it holds the list of column names.
> - */
> - sqlite3ColumnsFromExprList(pParse, pTable->pCheck,
> - &pTable->nCol,
> - &pTable->aCol);
> - if (db->mallocFailed == 0 && pParse->nErr == 0
> - && pTable->nCol == pSel->pEList->nExpr) {
> - sqlite3SelectAddColumnTypeAndCollation(pParse,
> - pTable,
> - pSel);
> - }
> - } else if (pSelTab) {
> - /* CREATE VIEW name AS... without an argument list. Construct
> - * the column names from the SELECT statement that defines the view.
> - */
> - assert(pTable->aCol == 0);
> - pTable->nCol = pSelTab->nCol;
> - pTable->aCol = pSelTab->aCol;
> - pSelTab->nCol = 0;
> - pSelTab->aCol = 0;
> - } else {
> - pTable->nCol = 0;
> - nErr++;
> + assert(table->pSelect != NULL);
> + sqlite3 *db = parse->db;
> + struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
> + if (select == NULL)
> + return false;
3. Same.
> + int n = parse->nTab;
> + sqlite3SrcListAssignCursors(parse, select->pSrc);
> + table->nCol = -1;
> + db->lookaside.bDisable++;
> + struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
> + parse->nTab = n;
> + bool is_ok = true;
4. is_ok -> rc.
> + if (table->pCheck != 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.
> + */
> + sqlite3ColumnsFromExprList(parse, table->pCheck,
> + &table->nCol,
> + &table->aCol);
> + if (db->mallocFailed == 0 && parse->nErr == 0
> + && table->nCol == select->pEList->nExpr) {
> + sqlite3SelectAddColumnTypeAndCollation(parse, table,
> + select);
> }
> - sqlite3DeleteTable(db, pSelTab);
> - sqlite3SelectDelete(db, pSel);
> - db->lookaside.bDisable--;
> + } 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);
> + table->nCol = sel_tab->nCol;
> + table->aCol = sel_tab->aCol;
> + sel_tab->nCol = 0;
> + sel_tab->aCol = NULL;
> } else {
> - nErr++;
> + table->nCol = 0;
> + is_ok = false;
> }
> -#endif /* SQLITE_OMIT_VIEW */
> - return nErr;
> + sqlite3DeleteTable(db, sel_tab);
> + sqlite3SelectDelete(db, select);
> + db->lookaside.bDisable--;
> +
> + return is_ok;
5. Same as 1 - 3.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-21 10:49 ` Vladislav Shpilevoy
@ 2018-05-21 11:24 ` Kirill Yukhin
2018-05-21 11:39 ` Vladislav Shpilevoy
0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2018-05-21 11:24 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On 21 мая 13:49, Vladislav Shpilevoy wrote:
>
>
> On 21/05/2018 13:25, Kirill Yukhin wrote:
> > On 21 мая 12:47, Vladislav Shpilevoy wrote:
> > > Hello. Thanks for the fixes!
> > >
> > > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> > > > index df75186..68f81e8 100644
> > > > --- a/src/box/sql/build.c
> > > > +++ b/src/box/sql/build.c
> > > > @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
> > > > -int
> > > > -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
> > > > +bool
> > >
> > > In Tarantool code style we do not return boolean on non-check functions.
> > > 0 or -1, please.
> > Done.
>
> No(( Tests are failing. See 5 comments below, why.
I've fixed that.
> > - if (pTable->nCol > 0)
> > - return 0;
> > + if (table->nCol > 0)
> > + return true;
>
> 1. Here you return 'true', but returned type is int. It is converted to 1,
> and this function is treated as failed.
Fixed.
> > + sqlite3ErrorMsg(parse, "view %s is circularly defined",
> > + table->zName);
> > + return false;
>
> 2. Same.
Ditto.
> > + if (select == NULL)
> > + return false;
>
> 3. Same.
Ditto.
> > + parse->nTab = n;
> > + bool is_ok = true;
>
> 4. is_ok -> rc.
Renamed.
> > + db->lookaside.bDisable--;
> > +
> > + return is_ok;
> 5. Same as 1 - 3.
Fixed.
--
Regards, Kirill Yukhin
commit 3bb55c57ade2eed862d43f03947dde7ee39357e3
Author: Kirill Yukhin <kyukhin@tarantool.org>
Date: Fri May 18 16:31:24 2018 +0300
sql: refactor SQL delete to allow Lua spaces
This is a first step toward fully-featured deletion
of spaces created in Lua by means of SQL language.
This change to handle most simple case:
DELETE * FROM <space_name> and will be improved in
nearest future.
Also, refactor (sqlite3ViewGetColumnNames()) to
use Tarantool's code style and naming convention.
Part of #3235
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 998dd5b..cc594a7 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -169,7 +169,8 @@ openStatTable(Parse * pParse, /* Parsing context */
* The sql_stat[134] table already exists.
* Delete all rows.
*/
- sqlite3VdbeAddOp2(v, OP_Clear, aRoot[i], 0);
+ sqlite3VdbeAddOp2(v, OP_Clear,
+ SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
}
}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index df75186..f81cb52 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
}
#endif /* SQLITE_OMIT_VIEW */
-#if !defined(SQLITE_OMIT_VIEW)
-/*
- * The Table structure pTable is really a VIEW. Fill in the names of
- * the columns of the view in the pTable structure. Return the number
- * of errors. If an error is seen leave an error message in pParse->zErrMsg.
- */
int
-sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
+sql_view_column_names(struct Parse *parse, struct Table *table)
{
- Table *pSelTab; /* A fake table from which we get the result set */
- Select *pSel; /* Copy of the SELECT that implements the view */
- int nErr = 0; /* Number of errors encountered */
- int n; /* Temporarily holds the number of cursors assigned */
- sqlite3 *db = pParse->db; /* Database connection for malloc errors */
-
- assert(pTable);
-
-#ifndef SQLITE_OMIT_VIEW
- /* A positive nCol means the columns names for this view are
- * already known.
+ assert(table != NULL);
+ assert(space_is_view(table));
+ /* A positive nCol means the columns names for this view
+ * are already known.
*/
- if (pTable->nCol > 0)
+ if (table->nCol > 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:
+ /* 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:
+ * 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 (pTable->nCol < 0) {
- sqlite3ErrorMsg(pParse, "view %s is circularly defined",
- pTable->zName);
- return 1;
- }
- assert(pTable->nCol >= 0);
-
- /* 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.
+ if (table->nCol < 0) {
+ sqlite3ErrorMsg(parse, "view %s is circularly defined",
+ table->zName);
+ return -1;
+ }
+ assert(table->nCol >= 0);
+
+ /* 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(pTable->pSelect);
- pSel = sqlite3SelectDup(db, pTable->pSelect, 0);
- if (pSel) {
- n = pParse->nTab;
- sqlite3SrcListAssignCursors(pParse, pSel->pSrc);
- pTable->nCol = -1;
- db->lookaside.bDisable++;
- pSelTab = sqlite3ResultSetOfSelect(pParse, pSel);
- pParse->nTab = n;
- if (pTable->pCheck) {
- /* CREATE VIEW name(arglist) AS ...
- * The names of the columns in the table are taken from
- * arglist which is stored in pTable->pCheck. The pCheck field
- * normally holds CHECK constraints on an ordinary table, but for
- * a VIEW it holds the list of column names.
- */
- sqlite3ColumnsFromExprList(pParse, pTable->pCheck,
- &pTable->nCol,
- &pTable->aCol);
- if (db->mallocFailed == 0 && pParse->nErr == 0
- && pTable->nCol == pSel->pEList->nExpr) {
- sqlite3SelectAddColumnTypeAndCollation(pParse,
- pTable,
- pSel);
- }
- } else if (pSelTab) {
- /* CREATE VIEW name AS... without an argument list. Construct
- * the column names from the SELECT statement that defines the view.
- */
- assert(pTable->aCol == 0);
- pTable->nCol = pSelTab->nCol;
- pTable->aCol = pSelTab->aCol;
- pSelTab->nCol = 0;
- pSelTab->aCol = 0;
- } else {
- pTable->nCol = 0;
- nErr++;
+ assert(table->pSelect != NULL);
+ sqlite3 *db = parse->db;
+ struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
+ if (select == NULL)
+ return -1;
+ int n = parse->nTab;
+ sqlite3SrcListAssignCursors(parse, select->pSrc);
+ table->nCol = -1;
+ db->lookaside.bDisable++;
+ struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
+ parse->nTab = n;
+ int rc = 0;
+ if (table->pCheck != 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.
+ */
+ sqlite3ColumnsFromExprList(parse, table->pCheck,
+ &table->nCol,
+ &table->aCol);
+ if (db->mallocFailed == 0 && parse->nErr == 0
+ && table->nCol == select->pEList->nExpr) {
+ sqlite3SelectAddColumnTypeAndCollation(parse, table,
+ select);
}
- sqlite3DeleteTable(db, pSelTab);
- sqlite3SelectDelete(db, pSel);
- db->lookaside.bDisable--;
+ } 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);
+ table->nCol = sel_tab->nCol;
+ table->aCol = sel_tab->aCol;
+ sel_tab->nCol = 0;
+ sel_tab->aCol = NULL;
} else {
- nErr++;
+ table->nCol = 0;
+ rc = -1;
}
-#endif /* SQLITE_OMIT_VIEW */
- return nErr;
+ sqlite3DeleteTable(db, sel_tab);
+ sqlite3SelectDelete(db, select);
+ db->lookaside.bDisable--;
+
+ return rc;
}
-#endif /* !defined(SQLITE_OMIT_VIEW) */
#ifndef SQLITE_OMIT_VIEW
/*
@@ -2672,7 +2662,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
VdbeCoverage(v);
sqlite3VdbeJumpHere(v, addr1);
if (memRootPage < 0)
- sqlite3VdbeAddOp2(v, OP_Clear, tnum, 0);
+ sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
+ 0);
emit_open_cursor(pParse, iIdx, tnum);
sqlite3VdbeChangeP5(v,
OPFLAG_BULKCSR | ((memRootPage >= 0) ?
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f4d248e..8c7cd7d 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
+#include "box/box.h"
#include "box/session.h"
#include "box/schema.h"
#include "sqliteInt.h"
@@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* with multiple tables and expect an SrcList* parameter
* instead of just a Table* parameter.
*/
- struct Table *table = sql_list_lookup_table(parse, tab_list);
- if (table == NULL)
- goto delete_from_cleanup;
-
- struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(table->tnum));
- assert(space != NULL);
+ struct Table *table = NULL;
+ struct space *space;
+ uint32_t space_id;
/* Figure out if we have any triggers and if the table
* being deleted from is a view.
*/
- struct Trigger *trigger_list = sqlite3TriggersExist(table, TK_DELETE,
- NULL, NULL);
-
- bool is_view = space->def->opts.is_view;
+ struct Trigger *trigger_list = NULL;
/* True if there are triggers or FKs or subqueries in the
* WHERE clause.
*/
- bool is_complex = trigger_list != NULL ||
- sqlite3FkRequired(table, NULL);
+ bool is_complex = false;
+ const char *tab_name = tab_list->a->zName;
+ if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
+ space_id = box_space_id_by_name(tab_name,
+ strlen(tab_name));
+ if (space_id == BOX_ID_NIL)
+ goto delete_from_cleanup;
+ } else {
+ table = sql_list_lookup_table(parse, tab_list);
+ if (table == NULL)
+ goto delete_from_cleanup;
+ space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+ trigger_list =sqlite3TriggersExist(table, TK_DELETE,
+ NULL, NULL);
+ is_complex = trigger_list != NULL ||
+ sqlite3FkRequired(table, NULL);
+ }
+ space = space_by_id(space_id);
+ assert(space != NULL);
+
+ bool is_view = space->def->opts.is_view;
/* If table is really a view, make sure it has been
* initialized.
*/
- if (sqlite3ViewGetColumnNames(parse, table))
- goto delete_from_cleanup;
+ if (is_view) {
+ if (sql_view_column_names(parse, table) != 0)
+ goto delete_from_cleanup;
- if (is_view && trigger_list == NULL) {
- sqlite3ErrorMsg(parse, "cannot modify %s because it is a view",
- space->def->name);
- goto delete_from_cleanup;
+ if (trigger_list == NULL) {
+ sqlite3ErrorMsg(parse, "cannot modify %s because it is a"
+ " view", space->def->name);
+ goto delete_from_cleanup;
+ }
}
/* Assign cursor numbers to the table and all its indices.
@@ -139,14 +155,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
tab_cursor);
}
- /* Resolve the column names in the WHERE clause. */
- struct NameContext nc;
- memset(&nc, 0, sizeof(nc));
- nc.pParse = parse;
- nc.pSrcList = tab_list;
- if (sqlite3ResolveExprNames(&nc, where))
- goto delete_from_cleanup;
-
/* Initialize the counter of the number of rows deleted,
* if we are counting rows.
*/
@@ -162,7 +170,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
if (where == NULL && !is_complex) {
assert(!is_view);
- sqlite3VdbeAddOp1(v, OP_Clear, table->tnum);
+ sqlite3VdbeAddOp1(v, OP_Clear, space_id);
/* Do not start Tarantool's transaction in case of
* truncate optimization. This is workaround until
@@ -171,6 +179,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
*/
parse->initiateTTrans = false;
} else {
+ /* Resolve the column names in the WHERE clause. */
+ struct NameContext nc;
+ memset(&nc, 0, sizeof(nc));
+ nc.pParse = parse;
+ nc.pSrcList = tab_list;
+ if (sqlite3ResolveExprNames(&nc, where))
+ goto delete_from_cleanup;
uint16_t wcf = WHERE_ONEPASS_DESIRED | WHERE_DUPLICATES_OK |
WHERE_SEEK_TABLE;
if (nc.ncFlags & NC_VarSelect)
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index e23cffd..5d62e58 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -349,11 +349,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
int *aRegIdx = 0; /* One register allocated to each index */
uint32_t space_id = 0;
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True if attempting to insert into a view */
+ bool is_view; /* True if attempting to insert into a view */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of trigger times */
-#endif
db = pParse->db;
memset(&dest, 0, sizeof(dest));
@@ -389,28 +387,18 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Figure out if we have any triggers and if the table being
* inserted into is a view
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_INSERT, 0, &tmask);
- isView = space_is_view(pTab);
-#else
-#define pTrigger 0
-#define tmask 0
-#define isView 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
+ is_view = space_is_view(pTab);
assert((pTrigger && tmask) || (pTrigger == 0 && tmask == 0));
/* If pTab is really a view, make sure it has been initialized.
* ViewGetColumnNames() is a no-op if pTab is not a view.
*/
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (is_view && sql_view_column_names(pParse, pTab) != 0)
goto insert_cleanup;
/* Cannot insert into a read-only table. */
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto insert_cleanup;
@@ -481,7 +469,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
bIdListInOrder = 0;
if (j == pTab->iPKey) {
ipkColumn = i;
- assert(isView);
+ assert(is_view);
}
break;
}
@@ -637,7 +625,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
}
/* If this is not a view, open the table and and all indices */
- if (!isView) {
+ if (!is_view) {
int nIdx;
nIdx =
sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0,
@@ -731,7 +719,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
* If this is a real table, attempt conversions as required by the
* table column affinities.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3TableAffinity(v, pTab, regCols + 1);
}
@@ -747,7 +735,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
/* Compute the content of the next row to insert into a range of
* registers beginning at regIns.
*/
- if (!isView) {
+ if (!is_view) {
if (ipkColumn >= 0) {
if (useTempTable) {
sqlite3VdbeAddOp3(v, OP_Column, srcTab,
@@ -930,20 +918,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */
sqlite3DbFree(db, aRegIdx);
}
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-#ifdef tmask
-#undef tmask
-#endif
-
/*
* Meanings of bits in of pWalker->eCode for checkConstraintUnchanged()
*/
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index e4ceec2..634da87 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -357,7 +357,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Column *pCol;
Index *pPk = sqlite3PrimaryKeyIndex(pTab);
pParse->nMem = 6;
- sqlite3ViewGetColumnNames(pParse, pTab);
+ if (space_is_view(pTab))
+ sql_view_column_names(pParse, pTab);
for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
i++, pCol++) {
if (!table_column_is_in_pk(pTab, i)) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index de111d3..e4d4013 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4680,7 +4680,7 @@ selectExpander(Walker * pWalker, Select * p)
#if !defined(SQLITE_OMIT_VIEW)
if (space_is_view(pTab)) {
i16 nCol;
- if (sqlite3ViewGetColumnNames(pParse, pTab))
+ if (sql_view_column_names(pParse, pTab) != 0)
return WRC_Abort;
assert(pFrom->pSelect == 0);
pFrom->pSelect =
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ab50c3e..26db466 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3592,11 +3592,18 @@ int sqlite3FaultSim(int);
void sqlite3CreateView(Parse *, Token *, Token *, ExprList *, Select *, int);
-#if !defined(SQLITE_OMIT_VIEW)
-int sqlite3ViewGetColumnNames(Parse *, Table *);
-#else
-#define sqlite3ViewGetColumnNames(A,B) 0
-#endif
+/**
+ * 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.
+ *
+ * @param parse Parsing context.
+ * @param table Tables to process.
+ * @retval 0 if success, -1 in case of error.
+ */
+int
+sql_view_column_names(struct Parse *parse, struct Table *table);
void
sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index c521a3b..4d6d0e1 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -137,11 +137,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
int labelContinue; /* Jump here to continue next step of UPDATE loop */
struct session *user_session = current_session();
-#ifndef SQLITE_OMIT_TRIGGER
- int isView; /* True when updating a view (INSTEAD OF trigger) */
+ bool is_view; /* True when updating a view (INSTEAD OF trigger) */
Trigger *pTrigger; /* List of triggers on pTab, if required */
int tmask; /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
-#endif
int newmask; /* Mask of NEW.* columns accessed by BEFORE triggers */
int iEph = 0; /* Ephemeral table holding all primary key values */
int nKey = 0; /* Number of elements in regKey */
@@ -170,24 +168,14 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* Figure out if we have any triggers and if the table being
* updated is a view.
*/
-#ifndef SQLITE_OMIT_TRIGGER
pTrigger = sqlite3TriggersExist(pTab, TK_UPDATE, pChanges, &tmask);
- isView = space_is_view(pTab);
+ is_view = space_is_view(pTab);
assert(pTrigger || tmask == 0);
-#else
-#define pTrigger 0
-#define isView 0
-#define tmask 0
-#endif
-#ifdef SQLITE_OMIT_VIEW
-#undef isView
-#define isView 0
-#endif
- if (sqlite3ViewGetColumnNames(pParse, pTab)) {
+ if (is_view && sql_view_column_names(pParse, pTab) != 0) {
goto update_cleanup;
}
- if (isView && tmask == 0) {
+ if (is_view && tmask == 0) {
sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
pTab->zName);
goto update_cleanup;
@@ -200,7 +188,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
*/
pTabList->a[0].iCursor = iBaseCur = iDataCur = pParse->nTab++;
iIdxCur = iDataCur + 1;
- pPk = isView ? 0 : sqlite3PrimaryKeyIndex(pTab);
+ pPk = is_view ? 0 : sqlite3PrimaryKeyIndex(pTab);
for (nIdx = 0, pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext, nIdx++) {
if (IsPrimaryKeyIndex(pIdx) && pPk != 0) {
iDataCur = pParse->nTab;
@@ -324,13 +312,11 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* If we are trying to update a view, realize that view into
* an ephemeral table.
*/
-#if !defined(SQLITE_OMIT_VIEW) && !defined(SQLITE_OMIT_TRIGGER)
- if (isView) {
+ if (is_view) {
sql_materialize_view(pParse, pTab->zName, pWhere, iDataCur);
/* Number of columns from SELECT plus ID.*/
nKey = pTab->nCol + 1;
}
-#endif
/* Resolve the column names in all the expressions in the
* WHERE clause.
@@ -347,7 +333,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
i16 nPk; /* Number of components of the PRIMARY KEY */
int addrOpen; /* Address of the OpenEphemeral instruction */
- if (isView) {
+ if (is_view) {
nPk = nKey;
} else {
assert(pPk != 0);
@@ -359,7 +345,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
iEph = pParse->nTab++;
sqlite3VdbeAddOp2(v, OP_Null, 0, iPk);
- if (isView) {
+ if (is_view) {
addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph,
nKey);
} else {
@@ -372,7 +358,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
if (pWInfo == 0)
goto update_cleanup;
okOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass);
- if (isView){
+ if (is_view) {
for (i = 0; i < nPk; i++) {
sqlite3VdbeAddOp3(v, OP_Column, iDataCur, i, iPk + i);
}
@@ -390,7 +376,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
nKey = nPk;
regKey = iPk;
} else {
- const char *zAff = isView ? 0 :
+ const char *zAff = is_view ? 0 :
sqlite3IndexAffinityStr(pParse->db, pPk);
sqlite3VdbeAddOp4(v, OP_MakeRecord, iPk, nPk, regKey,
zAff, nPk);
@@ -412,7 +398,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
labelBreak = sqlite3VdbeMakeLabel(v);
- if (!isView) {
+ if (!is_view) {
/*
* Open every index that needs updating. Note that if any
* index could potentially invoke a REPLACE conflict resolution
@@ -445,7 +431,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
labelContinue = labelBreak;
sqlite3VdbeAddOp2(v, OP_IsNull, regKey,
labelBreak);
- if (aToOpen[iDataCur - iBaseCur] && !isView) {
+ if (aToOpen[iDataCur - iBaseCur] && !is_view) {
assert(pPk);
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelBreak, regKey, nKey);
@@ -552,7 +538,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
* is deleted or renamed by a BEFORE trigger - is left undefined in the
* documentation.
*/
- if (!isView) {
+ if (!is_view) {
sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
labelContinue, regKey, nKey);
VdbeCoverage(v);
@@ -576,7 +562,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
}
- if (!isView) {
+ if (!is_view) {
int addr1 = 0; /* Address of jump instruction */
int bReplace = 0; /* True if REPLACE conflict resolution might happen */
@@ -691,15 +677,3 @@ sqlite3Update(Parse * pParse, /* The parser context */
sql_expr_free(db, pWhere, false);
return;
}
-
-/* Make sure "isView" and other macros defined above are undefined. Otherwise
- * they may interfere with compilation of other functions in this file
- * (or in another file, if this file becomes part of the amalgamation).
- */
-#ifdef isView
-#undef isView
-#endif
-#ifdef pTrigger
-#undef pTrigger
-#endif
-
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 455198d..9acb9b6 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4614,7 +4614,7 @@ case OP_IdxGE: { /* jump */
*/
case OP_Clear: {
assert(pOp->p1 > 0);
- uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pOp->p1);
+ uint32_t space_id = pOp->p1;
struct space *space = space_by_id(space_id);
assert(space != NULL);
rc = tarantoolSqlite3ClearTable(space);
diff --git a/test/sql-tap/delete1.test.lua b/test/sql-tap/delete1.test.lua
index 61d7756..810ca8a 100755
--- a/test/sql-tap/delete1.test.lua
+++ b/test/sql-tap/delete1.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(7)
+test:plan(9)
--!./tcltestrunner.lua
-- ["set","testdir",[["file","dirname",["argv0"]]]]
@@ -131,5 +131,26 @@ test:do_test(
-- </delete1-5.0>
})
+-- Tests for data dictionary integration.
+s = box.schema.create_space('t')
+i = s:create_index('i', {parts={1, 'unsigned'}})
+test:do_test(
+ "delete1-6.0",
+ function()
+ s:replace({1})
+ s:replace({2})
+ s:replace({3})
+ return s:count()
+ end,
+ 3)
+
+test:do_test(
+ "delete1-6.1",
+ function()
+ box.sql.execute([[delete from "t"]])
+ return s:count()
+ end,
+ 0)
+
test:finish_test()
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces
2018-05-21 11:24 ` Kirill Yukhin
@ 2018-05-21 11:39 ` Vladislav Shpilevoy
0 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-21 11:39 UTC (permalink / raw)
To: Kirill Yukhin; +Cc: tarantool-patches
Thanks! LGTM.
On 21/05/2018 14:24, Kirill Yukhin wrote:
> On 21 мая 13:49, Vladislav Shpilevoy wrote:
>>
>>
>> On 21/05/2018 13:25, Kirill Yukhin wrote:
>>> On 21 мая 12:47, Vladislav Shpilevoy wrote:
>>>> Hello. Thanks for the fixes!
>>>>
>>>>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>>>>> index df75186..68f81e8 100644
>>>>> --- a/src/box/sql/build.c
>>>>> +++ b/src/box/sql/build.c
>>>>> @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
>>>>> -int
>>>>> -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
>>>>> +bool
>>>>
>>>> In Tarantool code style we do not return boolean on non-check functions.
>>>> 0 or -1, please.
>>> Done.
>>
>> No(( Tests are failing. See 5 comments below, why.
> I've fixed that.
>
>>> - if (pTable->nCol > 0)
>>> - return 0;
>>> + if (table->nCol > 0)
>>> + return true;
>>
>> 1. Here you return 'true', but returned type is int. It is converted to 1,
>> and this function is treated as failed.
> Fixed.
>
>>> + sqlite3ErrorMsg(parse, "view %s is circularly defined",
>>> + table->zName);
>>> + return false;
>>
>> 2. Same.
> Ditto.
>
>>> + if (select == NULL)
>>> + return false;
>>
>> 3. Same.
> Ditto.
>
>>> + parse->nTab = n;
>>> + bool is_ok = true;
>>
>> 4. is_ok -> rc.
> Renamed.
>
>>> + db->lookaside.bDisable--;
>>> +
>>> + return is_ok;
>> 5. Same as 1 - 3.
> Fixed.
>
> --
> Regards, Kirill Yukhin
>
> commit 3bb55c57ade2eed862d43f03947dde7ee39357e3
> Author: Kirill Yukhin <kyukhin@tarantool.org>
> Date: Fri May 18 16:31:24 2018 +0300
>
> sql: refactor SQL delete to allow Lua spaces
>
> This is a first step toward fully-featured deletion
> of spaces created in Lua by means of SQL language.
> This change to handle most simple case:
> DELETE * FROM <space_name> and will be improved in
> nearest future.
> Also, refactor (sqlite3ViewGetColumnNames()) to
> use Tarantool's code style and naming convention.
>
> Part of #3235
>
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 998dd5b..cc594a7 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -169,7 +169,8 @@ openStatTable(Parse * pParse, /* Parsing context */
> * The sql_stat[134] table already exists.
> * Delete all rows.
> */
> - sqlite3VdbeAddOp2(v, OP_Clear, aRoot[i], 0);
> + sqlite3VdbeAddOp2(v, OP_Clear,
> + SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 0);
> }
> }
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index df75186..f81cb52 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2108,107 +2108,97 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
> }
> #endif /* SQLITE_OMIT_VIEW */
>
> -#if !defined(SQLITE_OMIT_VIEW)
> -/*
> - * The Table structure pTable is really a VIEW. Fill in the names of
> - * the columns of the view in the pTable structure. Return the number
> - * of errors. If an error is seen leave an error message in pParse->zErrMsg.
> - */
> int
> -sqlite3ViewGetColumnNames(Parse * pParse, Table * pTable)
> +sql_view_column_names(struct Parse *parse, struct Table *table)
> {
> - Table *pSelTab; /* A fake table from which we get the result set */
> - Select *pSel; /* Copy of the SELECT that implements the view */
> - int nErr = 0; /* Number of errors encountered */
> - int n; /* Temporarily holds the number of cursors assigned */
> - sqlite3 *db = pParse->db; /* Database connection for malloc errors */
> -
> - assert(pTable);
> -
> -#ifndef SQLITE_OMIT_VIEW
> - /* A positive nCol means the columns names for this view are
> - * already known.
> + assert(table != NULL);
> + assert(space_is_view(table));
> + /* A positive nCol means the columns names for this view
> + * are already known.
> */
> - if (pTable->nCol > 0)
> + if (table->nCol > 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:
> + /* 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:
> + * 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 (pTable->nCol < 0) {
> - sqlite3ErrorMsg(pParse, "view %s is circularly defined",
> - pTable->zName);
> - return 1;
> - }
> - assert(pTable->nCol >= 0);
> -
> - /* 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.
> + if (table->nCol < 0) {
> + sqlite3ErrorMsg(parse, "view %s is circularly defined",
> + table->zName);
> + return -1;
> + }
> + assert(table->nCol >= 0);
> +
> + /* 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(pTable->pSelect);
> - pSel = sqlite3SelectDup(db, pTable->pSelect, 0);
> - if (pSel) {
> - n = pParse->nTab;
> - sqlite3SrcListAssignCursors(pParse, pSel->pSrc);
> - pTable->nCol = -1;
> - db->lookaside.bDisable++;
> - pSelTab = sqlite3ResultSetOfSelect(pParse, pSel);
> - pParse->nTab = n;
> - if (pTable->pCheck) {
> - /* CREATE VIEW name(arglist) AS ...
> - * The names of the columns in the table are taken from
> - * arglist which is stored in pTable->pCheck. The pCheck field
> - * normally holds CHECK constraints on an ordinary table, but for
> - * a VIEW it holds the list of column names.
> - */
> - sqlite3ColumnsFromExprList(pParse, pTable->pCheck,
> - &pTable->nCol,
> - &pTable->aCol);
> - if (db->mallocFailed == 0 && pParse->nErr == 0
> - && pTable->nCol == pSel->pEList->nExpr) {
> - sqlite3SelectAddColumnTypeAndCollation(pParse,
> - pTable,
> - pSel);
> - }
> - } else if (pSelTab) {
> - /* CREATE VIEW name AS... without an argument list. Construct
> - * the column names from the SELECT statement that defines the view.
> - */
> - assert(pTable->aCol == 0);
> - pTable->nCol = pSelTab->nCol;
> - pTable->aCol = pSelTab->aCol;
> - pSelTab->nCol = 0;
> - pSelTab->aCol = 0;
> - } else {
> - pTable->nCol = 0;
> - nErr++;
> + assert(table->pSelect != NULL);
> + sqlite3 *db = parse->db;
> + struct Select *select = sqlite3SelectDup(db, table->pSelect, 0);
> + if (select == NULL)
> + return -1;
> + int n = parse->nTab;
> + sqlite3SrcListAssignCursors(parse, select->pSrc);
> + table->nCol = -1;
> + db->lookaside.bDisable++;
> + struct Table *sel_tab = sqlite3ResultSetOfSelect(parse, select);
> + parse->nTab = n;
> + int rc = 0;
> + if (table->pCheck != 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.
> + */
> + sqlite3ColumnsFromExprList(parse, table->pCheck,
> + &table->nCol,
> + &table->aCol);
> + if (db->mallocFailed == 0 && parse->nErr == 0
> + && table->nCol == select->pEList->nExpr) {
> + sqlite3SelectAddColumnTypeAndCollation(parse, table,
> + select);
> }
> - sqlite3DeleteTable(db, pSelTab);
> - sqlite3SelectDelete(db, pSel);
> - db->lookaside.bDisable--;
> + } 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);
> + table->nCol = sel_tab->nCol;
> + table->aCol = sel_tab->aCol;
> + sel_tab->nCol = 0;
> + sel_tab->aCol = NULL;
> } else {
> - nErr++;
> + table->nCol = 0;
> + rc = -1;
> }
> -#endif /* SQLITE_OMIT_VIEW */
> - return nErr;
> + sqlite3DeleteTable(db, sel_tab);
> + sqlite3SelectDelete(db, select);
> + db->lookaside.bDisable--;
> +
> + return rc;
> }
> -#endif /* !defined(SQLITE_OMIT_VIEW) */
>
> #ifndef SQLITE_OMIT_VIEW
> /*
> @@ -2672,7 +2662,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
> VdbeCoverage(v);
> sqlite3VdbeJumpHere(v, addr1);
> if (memRootPage < 0)
> - sqlite3VdbeAddOp2(v, OP_Clear, tnum, 0);
> + sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
> + 0);
> emit_open_cursor(pParse, iIdx, tnum);
> sqlite3VdbeChangeP5(v,
> OPFLAG_BULKCSR | ((memRootPage >= 0) ?
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index f4d248e..8c7cd7d 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -29,6 +29,7 @@
> * SUCH DAMAGE.
> */
>
> +#include "box/box.h"
> #include "box/session.h"
> #include "box/schema.h"
> #include "sqliteInt.h"
> @@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> * with multiple tables and expect an SrcList* parameter
> * instead of just a Table* parameter.
> */
> - struct Table *table = sql_list_lookup_table(parse, tab_list);
> - if (table == NULL)
> - goto delete_from_cleanup;
> -
> - struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(table->tnum));
> - assert(space != NULL);
> + struct Table *table = NULL;
> + struct space *space;
> + uint32_t space_id;
> /* Figure out if we have any triggers and if the table
> * being deleted from is a view.
> */
> - struct Trigger *trigger_list = sqlite3TriggersExist(table, TK_DELETE,
> - NULL, NULL);
> -
> - bool is_view = space->def->opts.is_view;
> + struct Trigger *trigger_list = NULL;
> /* True if there are triggers or FKs or subqueries in the
> * WHERE clause.
> */
> - bool is_complex = trigger_list != NULL ||
> - sqlite3FkRequired(table, NULL);
> + bool is_complex = false;
> + const char *tab_name = tab_list->a->zName;
> + if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
> + space_id = box_space_id_by_name(tab_name,
> + strlen(tab_name));
> + if (space_id == BOX_ID_NIL)
> + goto delete_from_cleanup;
> + } else {
> + table = sql_list_lookup_table(parse, tab_list);
> + if (table == NULL)
> + goto delete_from_cleanup;
> + space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
> + trigger_list =sqlite3TriggersExist(table, TK_DELETE,
> + NULL, NULL);
> + is_complex = trigger_list != NULL ||
> + sqlite3FkRequired(table, NULL);
> + }
> + space = space_by_id(space_id);
> + assert(space != NULL);
> +
> + bool is_view = space->def->opts.is_view;
>
> /* If table is really a view, make sure it has been
> * initialized.
> */
> - if (sqlite3ViewGetColumnNames(parse, table))
> - goto delete_from_cleanup;
> + if (is_view) {
> + if (sql_view_column_names(parse, table) != 0)
> + goto delete_from_cleanup;
>
> - if (is_view && trigger_list == NULL) {
> - sqlite3ErrorMsg(parse, "cannot modify %s because it is a view",
> - space->def->name);
> - goto delete_from_cleanup;
> + if (trigger_list == NULL) {
> + sqlite3ErrorMsg(parse, "cannot modify %s because it is a"
> + " view", space->def->name);
> + goto delete_from_cleanup;
> + }
> }
>
> /* Assign cursor numbers to the table and all its indices.
> @@ -139,14 +155,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> tab_cursor);
> }
>
> - /* Resolve the column names in the WHERE clause. */
> - struct NameContext nc;
> - memset(&nc, 0, sizeof(nc));
> - nc.pParse = parse;
> - nc.pSrcList = tab_list;
> - if (sqlite3ResolveExprNames(&nc, where))
> - goto delete_from_cleanup;
> -
> /* Initialize the counter of the number of rows deleted,
> * if we are counting rows.
> */
> @@ -162,7 +170,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> if (where == NULL && !is_complex) {
> assert(!is_view);
>
> - sqlite3VdbeAddOp1(v, OP_Clear, table->tnum);
> + sqlite3VdbeAddOp1(v, OP_Clear, space_id);
>
> /* Do not start Tarantool's transaction in case of
> * truncate optimization. This is workaround until
> @@ -171,6 +179,13 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> */
> parse->initiateTTrans = false;
> } else {
> + /* Resolve the column names in the WHERE clause. */
> + struct NameContext nc;
> + memset(&nc, 0, sizeof(nc));
> + nc.pParse = parse;
> + nc.pSrcList = tab_list;
> + if (sqlite3ResolveExprNames(&nc, where))
> + goto delete_from_cleanup;
> uint16_t wcf = WHERE_ONEPASS_DESIRED | WHERE_DUPLICATES_OK |
> WHERE_SEEK_TABLE;
> if (nc.ncFlags & NC_VarSelect)
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index e23cffd..5d62e58 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -349,11 +349,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> int *aRegIdx = 0; /* One register allocated to each index */
> uint32_t space_id = 0;
>
> -#ifndef SQLITE_OMIT_TRIGGER
> - int isView; /* True if attempting to insert into a view */
> + bool is_view; /* True if attempting to insert into a view */
> Trigger *pTrigger; /* List of triggers on pTab, if required */
> int tmask; /* Mask of trigger times */
> -#endif
>
> db = pParse->db;
> memset(&dest, 0, sizeof(dest));
> @@ -389,28 +387,18 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> /* Figure out if we have any triggers and if the table being
> * inserted into is a view
> */
> -#ifndef SQLITE_OMIT_TRIGGER
> pTrigger = sqlite3TriggersExist(pTab, TK_INSERT, 0, &tmask);
> - isView = space_is_view(pTab);
> -#else
> -#define pTrigger 0
> -#define tmask 0
> -#define isView 0
> -#endif
> -#ifdef SQLITE_OMIT_VIEW
> -#undef isView
> -#define isView 0
> -#endif
> + is_view = space_is_view(pTab);
> assert((pTrigger && tmask) || (pTrigger == 0 && tmask == 0));
>
> /* If pTab is really a view, make sure it has been initialized.
> * ViewGetColumnNames() is a no-op if pTab is not a view.
> */
> - if (sqlite3ViewGetColumnNames(pParse, pTab))
> + if (is_view && sql_view_column_names(pParse, pTab) != 0)
> goto insert_cleanup;
>
> /* Cannot insert into a read-only table. */
> - if (isView && tmask == 0) {
> + if (is_view && tmask == 0) {
> sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
> pTab->zName);
> goto insert_cleanup;
> @@ -481,7 +469,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> bIdListInOrder = 0;
> if (j == pTab->iPKey) {
> ipkColumn = i;
> - assert(isView);
> + assert(is_view);
> }
> break;
> }
> @@ -637,7 +625,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> }
>
> /* If this is not a view, open the table and and all indices */
> - if (!isView) {
> + if (!is_view) {
> int nIdx;
> nIdx =
> sqlite3OpenTableAndIndices(pParse, pTab, OP_OpenWrite, 0,
> @@ -731,7 +719,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> * If this is a real table, attempt conversions as required by the
> * table column affinities.
> */
> - if (!isView) {
> + if (!is_view) {
> sqlite3TableAffinity(v, pTab, regCols + 1);
> }
>
> @@ -747,7 +735,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> /* Compute the content of the next row to insert into a range of
> * registers beginning at regIns.
> */
> - if (!isView) {
> + if (!is_view) {
> if (ipkColumn >= 0) {
> if (useTempTable) {
> sqlite3VdbeAddOp3(v, OP_Column, srcTab,
> @@ -930,20 +918,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */
> sqlite3DbFree(db, aRegIdx);
> }
>
> -/* Make sure "isView" and other macros defined above are undefined. Otherwise
> - * they may interfere with compilation of other functions in this file
> - * (or in another file, if this file becomes part of the amalgamation).
> - */
> -#ifdef isView
> -#undef isView
> -#endif
> -#ifdef pTrigger
> -#undef pTrigger
> -#endif
> -#ifdef tmask
> -#undef tmask
> -#endif
> -
> /*
> * Meanings of bits in of pWalker->eCode for checkConstraintUnchanged()
> */
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index e4ceec2..634da87 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -357,7 +357,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
> Column *pCol;
> Index *pPk = sqlite3PrimaryKeyIndex(pTab);
> pParse->nMem = 6;
> - sqlite3ViewGetColumnNames(pParse, pTab);
> + if (space_is_view(pTab))
> + sql_view_column_names(pParse, pTab);
> for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
> i++, pCol++) {
> if (!table_column_is_in_pk(pTab, i)) {
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index de111d3..e4d4013 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -4680,7 +4680,7 @@ selectExpander(Walker * pWalker, Select * p)
> #if !defined(SQLITE_OMIT_VIEW)
> if (space_is_view(pTab)) {
> i16 nCol;
> - if (sqlite3ViewGetColumnNames(pParse, pTab))
> + if (sql_view_column_names(pParse, pTab) != 0)
> return WRC_Abort;
> assert(pFrom->pSelect == 0);
> pFrom->pSelect =
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index ab50c3e..26db466 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -3592,11 +3592,18 @@ int sqlite3FaultSim(int);
>
> void sqlite3CreateView(Parse *, Token *, Token *, ExprList *, Select *, int);
>
> -#if !defined(SQLITE_OMIT_VIEW)
> -int sqlite3ViewGetColumnNames(Parse *, Table *);
> -#else
> -#define sqlite3ViewGetColumnNames(A,B) 0
> -#endif
> +/**
> + * 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.
> + *
> + * @param parse Parsing context.
> + * @param table Tables to process.
> + * @retval 0 if success, -1 in case of error.
> + */
> +int
> +sql_view_column_names(struct Parse *parse, struct Table *table);
>
> void
> sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
> diff --git a/src/box/sql/update.c b/src/box/sql/update.c
> index c521a3b..4d6d0e1 100644
> --- a/src/box/sql/update.c
> +++ b/src/box/sql/update.c
> @@ -137,11 +137,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
> int labelContinue; /* Jump here to continue next step of UPDATE loop */
> struct session *user_session = current_session();
>
> -#ifndef SQLITE_OMIT_TRIGGER
> - int isView; /* True when updating a view (INSTEAD OF trigger) */
> + bool is_view; /* True when updating a view (INSTEAD OF trigger) */
> Trigger *pTrigger; /* List of triggers on pTab, if required */
> int tmask; /* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
> -#endif
> int newmask; /* Mask of NEW.* columns accessed by BEFORE triggers */
> int iEph = 0; /* Ephemeral table holding all primary key values */
> int nKey = 0; /* Number of elements in regKey */
> @@ -170,24 +168,14 @@ sqlite3Update(Parse * pParse, /* The parser context */
> /* Figure out if we have any triggers and if the table being
> * updated is a view.
> */
> -#ifndef SQLITE_OMIT_TRIGGER
> pTrigger = sqlite3TriggersExist(pTab, TK_UPDATE, pChanges, &tmask);
> - isView = space_is_view(pTab);
> + is_view = space_is_view(pTab);
> assert(pTrigger || tmask == 0);
> -#else
> -#define pTrigger 0
> -#define isView 0
> -#define tmask 0
> -#endif
> -#ifdef SQLITE_OMIT_VIEW
> -#undef isView
> -#define isView 0
> -#endif
>
> - if (sqlite3ViewGetColumnNames(pParse, pTab)) {
> + if (is_view && sql_view_column_names(pParse, pTab) != 0) {
> goto update_cleanup;
> }
> - if (isView && tmask == 0) {
> + if (is_view && tmask == 0) {
> sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
> pTab->zName);
> goto update_cleanup;
> @@ -200,7 +188,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> */
> pTabList->a[0].iCursor = iBaseCur = iDataCur = pParse->nTab++;
> iIdxCur = iDataCur + 1;
> - pPk = isView ? 0 : sqlite3PrimaryKeyIndex(pTab);
> + pPk = is_view ? 0 : sqlite3PrimaryKeyIndex(pTab);
> for (nIdx = 0, pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext, nIdx++) {
> if (IsPrimaryKeyIndex(pIdx) && pPk != 0) {
> iDataCur = pParse->nTab;
> @@ -324,13 +312,11 @@ sqlite3Update(Parse * pParse, /* The parser context */
> /* If we are trying to update a view, realize that view into
> * an ephemeral table.
> */
> -#if !defined(SQLITE_OMIT_VIEW) && !defined(SQLITE_OMIT_TRIGGER)
> - if (isView) {
> + if (is_view) {
> sql_materialize_view(pParse, pTab->zName, pWhere, iDataCur);
> /* Number of columns from SELECT plus ID.*/
> nKey = pTab->nCol + 1;
> }
> -#endif
>
> /* Resolve the column names in all the expressions in the
> * WHERE clause.
> @@ -347,7 +333,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> i16 nPk; /* Number of components of the PRIMARY KEY */
> int addrOpen; /* Address of the OpenEphemeral instruction */
>
> - if (isView) {
> + if (is_view) {
> nPk = nKey;
> } else {
> assert(pPk != 0);
> @@ -359,7 +345,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> iEph = pParse->nTab++;
> sqlite3VdbeAddOp2(v, OP_Null, 0, iPk);
>
> - if (isView) {
> + if (is_view) {
> addrOpen = sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, iEph,
> nKey);
> } else {
> @@ -372,7 +358,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> if (pWInfo == 0)
> goto update_cleanup;
> okOnePass = sqlite3WhereOkOnePass(pWInfo, aiCurOnePass);
> - if (isView){
> + if (is_view) {
> for (i = 0; i < nPk; i++) {
> sqlite3VdbeAddOp3(v, OP_Column, iDataCur, i, iPk + i);
> }
> @@ -390,7 +376,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> nKey = nPk;
> regKey = iPk;
> } else {
> - const char *zAff = isView ? 0 :
> + const char *zAff = is_view ? 0 :
> sqlite3IndexAffinityStr(pParse->db, pPk);
> sqlite3VdbeAddOp4(v, OP_MakeRecord, iPk, nPk, regKey,
> zAff, nPk);
> @@ -412,7 +398,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> }
>
> labelBreak = sqlite3VdbeMakeLabel(v);
> - if (!isView) {
> + if (!is_view) {
> /*
> * Open every index that needs updating. Note that if any
> * index could potentially invoke a REPLACE conflict resolution
> @@ -445,7 +431,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> labelContinue = labelBreak;
> sqlite3VdbeAddOp2(v, OP_IsNull, regKey,
> labelBreak);
> - if (aToOpen[iDataCur - iBaseCur] && !isView) {
> + if (aToOpen[iDataCur - iBaseCur] && !is_view) {
> assert(pPk);
> sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
> labelBreak, regKey, nKey);
> @@ -552,7 +538,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> * is deleted or renamed by a BEFORE trigger - is left undefined in the
> * documentation.
> */
> - if (!isView) {
> + if (!is_view) {
> sqlite3VdbeAddOp4Int(v, OP_NotFound, iDataCur,
> labelContinue, regKey, nKey);
> VdbeCoverage(v);
> @@ -576,7 +562,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
> }
> }
>
> - if (!isView) {
> + if (!is_view) {
> int addr1 = 0; /* Address of jump instruction */
> int bReplace = 0; /* True if REPLACE conflict resolution might happen */
>
> @@ -691,15 +677,3 @@ sqlite3Update(Parse * pParse, /* The parser context */
> sql_expr_free(db, pWhere, false);
> return;
> }
> -
> -/* Make sure "isView" and other macros defined above are undefined. Otherwise
> - * they may interfere with compilation of other functions in this file
> - * (or in another file, if this file becomes part of the amalgamation).
> - */
> -#ifdef isView
> -#undef isView
> -#endif
> -#ifdef pTrigger
> -#undef pTrigger
> -#endif
> -
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 455198d..9acb9b6 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -4614,7 +4614,7 @@ case OP_IdxGE: { /* jump */
> */
> case OP_Clear: {
> assert(pOp->p1 > 0);
> - uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pOp->p1);
> + uint32_t space_id = pOp->p1;
> struct space *space = space_by_id(space_id);
> assert(space != NULL);
> rc = tarantoolSqlite3ClearTable(space);
> diff --git a/test/sql-tap/delete1.test.lua b/test/sql-tap/delete1.test.lua
> index 61d7756..810ca8a 100755
> --- a/test/sql-tap/delete1.test.lua
> +++ b/test/sql-tap/delete1.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(7)
> +test:plan(9)
>
> --!./tcltestrunner.lua
> -- ["set","testdir",[["file","dirname",["argv0"]]]]
> @@ -131,5 +131,26 @@ test:do_test(
> -- </delete1-5.0>
> })
>
> +-- Tests for data dictionary integration.
> +s = box.schema.create_space('t')
> +i = s:create_index('i', {parts={1, 'unsigned'}})
> +test:do_test(
> + "delete1-6.0",
> + function()
> + s:replace({1})
> + s:replace({2})
> + s:replace({3})
> + return s:count()
> + end,
> + 3)
> +
> +test:do_test(
> + "delete1-6.1",
> + function()
> + box.sql.execute([[delete from "t"]])
> + return s:count()
> + end,
> + 0)
> +
> test:finish_test()
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-21 11:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-18 13:36 [tarantool-patches] [PATCH] sql: refactor SQL delete to allow Lua spaces Kirill Yukhin
2018-05-18 21:07 ` [tarantool-patches] " Vladislav Shpilevoy
2018-05-21 8:20 ` Kirill Yukhin
2018-05-21 9:47 ` Vladislav Shpilevoy
2018-05-21 10:25 ` Kirill Yukhin
2018-05-21 10:49 ` Vladislav Shpilevoy
2018-05-21 11:24 ` Kirill Yukhin
2018-05-21 11:39 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox