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 C558923B5B for ; Mon, 21 May 2018 06:49:34 -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 3rr7PrrrFffW for ; Mon, 21 May 2018 06:49:34 -0400 (EDT) Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 19A9122B17 for ; Mon, 21 May 2018 06:49:33 -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> From: Vladislav Shpilevoy Message-ID: Date: Mon, 21 May 2018 13:49:31 +0300 MIME-Version: 1.0 In-Reply-To: <20180521102537.22b4vvsyikwe62im@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 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.