From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 9CB6D21B70 for ; Mon, 21 May 2018 07:39:58 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Agn3RHCmLiaG for ; Mon, 21 May 2018 07:39:58 -0400 (EDT) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 329C821B42 for ; Mon, 21 May 2018 07:39:58 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces References: <87358913270ee98c98bc276209289d6b7ac496b6.1526650490.git.kyukhin@tarantool.org> <946fafd7-9e8a-0263-ecaa-02df83c82223@tarantool.org> <20180521082048.2n5vzzsgxcb7fyss@tarantool.org> <20180521102537.22b4vvsyikwe62im@tarantool.org> <20180521112459.7keo3phosmhgrbp6@tarantool.org> From: Vladislav Shpilevoy Message-ID: <182e28e6-3582-56ac-d7bd-8e07d0a7f547@tarantool.org> Date: Mon, 21 May 2018 14:39:53 +0300 MIME-Version: 1.0 In-Reply-To: <20180521112459.7keo3phosmhgrbp6@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Kirill Yukhin Cc: tarantool-patches@freelists.org 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 > 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 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( > -- > }) > > +-- 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() > >