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 11A262D6E0 for ; Wed, 18 Apr 2018 05:47:39 -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 mTvdREFBOIyq for ; Wed, 18 Apr 2018 05:47:39 -0400 (EDT) Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (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 0E4BF2D670 for ; Wed, 18 Apr 2018 05:47:37 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] Removed Expr pointer from SQL Column structure. References: <60fd0daec8e35563602212ca8a3307ab49b14d15.1523896524.git.kshcherbatov@tarantool.org> <71db35b8-be0a-34be-07ab-09d63c4c39cd@tarantool.org> <07b96d39-dcd4-9900-defe-3f11b9b20537@tarantool.org> <951402e9-502b-d1c2-e9ef-9805bd56ac90@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5eaf0628-8f05-eb95-1a4c-76b69e66cc00@tarantool.org> Date: Wed, 18 Apr 2018 12:47:33 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: tarantool-patches@freelists.org, Nikita Pettik Cc: Kirill Shcherbatov Hello. Thanks for fixes. I force pushed a pair of new ones into the same commit. Nikita, please, review the patch. diff --git a/src/box/sql.c b/src/box/sql.c index a6713f1f0..6bc82c280 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -1466,7 +1466,8 @@ int tarantoolSqlite3MakeTableFormat(Table *pTable, void *buf) for (i = 0; i < n; i++) { const char *t; struct coll *coll = NULL; - struct Expr *def = aCol[i].pDflt; + struct field_def *field = &pTable->def->fields[i]; + struct Expr *def = field->default_value_expr; if (aCol[i].zColl != NULL && strcasecmp(aCol[i].zColl, "binary") != 0) { coll = sqlite3FindCollSeq(aCol[i].zColl); diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c index 129ef823c..cf8be5d41 100644 --- a/src/box/sql/alter.c +++ b/src/box/sql/alter.c @@ -35,6 +35,7 @@ */ #include "sqliteInt.h" #include "src/box/session.h" +#include "tarantoolInt.h" /* * The code in this file only exists if we are not omitting the @@ -161,7 +162,9 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef) zTab = &pNew->zName[16]; /* Skip the "sqlite_altertab_" prefix on the name */ pCol = &pNew->aCol[pNew->nCol - 1]; - pDflt = pCol->pDflt; + assert(pNew->def != NULL); + pDflt = space_column_default_expr(SQLITE_PAGENO_TO_SPACEID(pNew->tnum), + pNew->nCol - 1); pTab = sqlite3HashFind(&db->pSchema->tblHash, zTab);; assert(pTab); @@ -297,7 +300,6 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc) Column *pCol = &pNew->aCol[i]; pCol->zName = sqlite3DbStrDup(db, pCol->zName); pCol->zColl = 0; - pCol->pDflt = 0; } pNew->pSchema = db->pSchema; pNew->addColOffset = pTab->addColOffset; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 92f3cb607..ab6df467d 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -299,7 +299,6 @@ sqlite3DeleteColumnNames(sqlite3 * db, Table * pTable) if ((pCol = pTable->aCol) != 0) { for (i = 0; i < pTable->nCol; i++, pCol++) { sqlite3DbFree(db, pCol->zName); - sql_expr_free(db, pCol->pDflt, false); sqlite3DbFree(db, pCol->zColl); } sqlite3DbFree(db, pTable->aCol); @@ -397,6 +396,12 @@ deleteTable(sqlite3 * db, Table * pTable) sqlite3DbFree(db, pTable->zColAff); sqlite3SelectDelete(db, pTable->pSelect); sqlite3ExprListDelete(db, pTable->pCheck); + if (pTable->def != NULL) { + /* Fields has been allocated independently. */ + struct field_def *fields = pTable->def->fields; + space_def_delete(pTable->def); + sqlite3DbFree(db, fields); + } sqlite3DbFree(db, pTable); /* Verify that no lookaside memory was used by schema tables */ @@ -490,6 +495,43 @@ sqlite3PrimaryKeyIndex(Table * pTab) return p; } +/** + * Create and initialize a new SQL Table object. + * @param pParse SQL Parser object. + * @param zName Table to create name. + * @retval NULL on memory allocation error, Parser state is + * changed. + * @retval not NULL on success. + */ +static Table * +sql_table_new(Parse *pParse, char *zName) +{ + sqlite3 *db = pParse->db; + + Table *pTable = sqlite3DbMallocZero(db, sizeof(Table)); + struct space_def *def = space_def_new(0, 0, 0, NULL, 0, NULL, 0, + &space_opts_default, NULL, 0); + if (pTable == NULL || def == NULL) { + if (def != NULL) + space_def_delete(def); + sqlite3DbFree(db, pTable); + pParse->rc = SQLITE_NOMEM_BKPT; + pParse->nErr++; + return NULL; + } + + pTable->def = def; + pTable->zName = zName; + pTable->iPKey = -1; + pTable->iAutoIncPKey = -1; + pTable->pSchema = db->pSchema; + sqlite3HashInit(&pTable->idxHash); + pTable->nTabRef = 1; + pTable->nRowLogEst = 200; + assert(200 == sqlite3LogEst(1048576)); + return pTable; +} + /* * Begin constructing a new table representation in memory. This is * the first of several action routines that get called in response @@ -547,21 +589,10 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr) goto begin_table_error; } - pTable = sqlite3DbMallocZero(db, sizeof(Table)); - if (pTable == 0) { - assert(db->mallocFailed); - pParse->rc = SQLITE_NOMEM_BKPT; - pParse->nErr++; + pTable = sql_table_new(pParse, zName); + if (pTable == NULL) goto begin_table_error; - } - pTable->zName = zName; - pTable->iPKey = -1; - pTable->iAutoIncPKey = -1; - pTable->pSchema = db->pSchema; - sqlite3HashInit(&pTable->idxHash); - pTable->nTabRef = 1; - pTable->nRowLogEst = 200; - assert(200 == sqlite3LogEst(1048576)); + assert(pParse->pNewTable == 0); pParse->pNewTable = pTable; @@ -585,6 +616,47 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr) return; } +/** + * Get field by id. Allocate memory if needed. + * @param pParse SQL Parser object. + * @param pTable SQL Table object. + * @param id column identifier. + * @retval not NULL on success. + * @retval NULL on out of memory. + */ +static struct field_def * +sql_field_retrieve(Parse *pParse, Table *pTable, uint32_t id) +{ + sqlite3 *db = pParse->db; + struct field_def *field; + assert(pTable->def); + assert(pTable->def->exact_field_count >= (uint32_t)pTable->nCol); + assert(id < (uint32_t)db->aLimit[SQLITE_LIMIT_COLUMN]); + + if (id >= pTable->def->exact_field_count) { + uint32_t nCol = pTable->def->exact_field_count; + nCol = (nCol > 0) ? 2 * nCol : 1; + field = sqlite3DbRealloc(db, pTable->def->fields, + nCol * sizeof(pTable->def->fields[0])); + if (field == NULL) { + pParse->rc = SQLITE_NOMEM_BKPT; + pParse->nErr++; + return NULL; + } + + for (uint32_t i = nCol / 2; i < nCol; i++) { + memcpy(&field[i], &field_def_default, + sizeof(struct field_def)); + } + + pTable->def->fields = field; + pTable->def->exact_field_count = nCol; + } + + field = &pTable->def->fields[id]; + return field; +} + /* * Add a new column to the table currently being constructed. * @@ -610,6 +682,8 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType) return; } #endif + if (sql_field_retrieve(pParse, p, (uint32_t) p->nCol) == NULL) + return; z = sqlite3DbMallocRaw(db, pName->n + 1); if (z == 0) return; @@ -668,6 +742,7 @@ sqlite3AddColumn(Parse * pParse, Token * pName, Token * pType) } } p->nCol++; + p->def->field_count++; pParse->constraintName.n = 0; } @@ -813,7 +888,11 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan) * is required by pragma table_info. */ Expr x; - sql_expr_free(db, pCol->pDflt, false); + assert(p->def != NULL); + struct field_def *field = + &p->def->fields[p->nCol - 1]; + sql_expr_free(db, field->default_value_expr, false); + memset(&x, 0, sizeof(x)); x.op = TK_SPAN; x.u.zToken = sqlite3DbStrNDup(db, (char *)pSpan->zStart, @@ -821,7 +900,9 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan) pSpan->zStart)); x.pLeft = pSpan->pExpr; x.flags = EP_Skip; - pCol->pDflt = sqlite3ExprDup(db, &x, EXPRDUP_REDUCE); + + field->default_value_expr = + sqlite3ExprDup(db, &x, EXPRDUP_REDUCE); sqlite3DbFree(db, x.u.zToken); } } diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c index f56b6d9cd..9b88b5f3a 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -36,6 +36,7 @@ #include #include "sqliteInt.h" #include "box/session.h" +#include "tarantoolInt.h" #ifndef SQLITE_OMIT_FOREIGN_KEY #ifndef SQLITE_OMIT_TRIGGER @@ -1346,8 +1347,12 @@ fkActionTrigger(Parse * pParse, /* Parse context */ &tToCol, 0)); } else if (action == OE_SetDflt) { + uint32_t space_id = + SQLITE_PAGENO_TO_SPACEID( + pFKey->pFrom->tnum); Expr *pDflt = - pFKey->pFrom->aCol[iFromCol].pDflt; + space_column_default_expr( + space_id, (uint32_t)iFromCol); if (pDflt) { pNew = sqlite3ExprDup(db, pDflt, diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index b24d55b07..067f50a0a 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1801,14 +1801,21 @@ xferOptimization(Parse * pParse, /* Parser context */ } /* Default values for second and subsequent columns need to match. */ if (i > 0) { - assert(pDestCol->pDflt == 0 - || pDestCol->pDflt->op == TK_SPAN); - assert(pSrcCol->pDflt == 0 - || pSrcCol->pDflt->op == TK_SPAN); - if ((pDestCol->pDflt == 0) != (pSrcCol->pDflt == 0) - || (pDestCol->pDflt - && strcmp(pDestCol->pDflt->u.zToken, - pSrcCol->pDflt->u.zToken) != 0) + uint32_t src_space_id = + SQLITE_PAGENO_TO_SPACEID(pSrc->tnum); + struct space *src_space = + space_cache_find(src_space_id); + uint32_t dest_space_id = + SQLITE_PAGENO_TO_SPACEID(pDest->tnum); + struct space *dest_space = + space_cache_find(dest_space_id); + char *src_expr_str = + src_space->def->fields[i].default_value; + char *dest_expr_str = + dest_space->def->fields[i].default_value; + if ((dest_expr_str == NULL) != (src_expr_str == NULL) + || (dest_expr_str + && strcmp(src_expr_str, dest_expr_str) != 0) ) { return 0; /* Default values must be the same for all columns */ } diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index b724c9845..3ca94a1c3 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -36,6 +36,7 @@ #include #include #include +#include "box/schema.h" #include "sqliteInt.h" #include "vdbeInt.h" #include "box/session.h" @@ -64,6 +65,7 @@ * ../tool/mkpragmatab.tcl. */ #include "pragma.h" +#include "tarantoolInt.h" /* * Interpret the given string as a safety level. Return 0 for OFF, @@ -370,19 +372,21 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ i; k++) { } } - assert(pCol->pDflt == 0 - || pCol->pDflt->op == TK_SPAN); bool nullable = table_column_is_nullable(pTab, i); + uint32_t space_id = + SQLITE_PAGENO_TO_SPACEID( + pTab->tnum); + struct space *space = + space_cache_find(space_id); + char *expr_str = space-> + def->fields[i].default_value; sqlite3VdbeMultiLoad(v, 1, "issisi", i, pCol->zName, field_type_strs[ sqlite3ColumnType (pCol)], nullable == 0, - pCol-> - pDflt ? pCol-> - pDflt->u.zToken - : 0, k); + expr_str, k); sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 6); } diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 59662cf14..7ce5bac5d 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1877,7 +1877,6 @@ struct Savepoint { struct Column { char *zName; /* Name of this column */ enum field_type type; /* Column type. */ - Expr *pDflt; /* Default value of this column */ char *zColl; /* Collating sequence. If NULL, use the default */ enum on_conflict_action notNull; /* An ON_CONFLICT_ACTION code for * handling a NOT NULL constraint @@ -1970,6 +1969,8 @@ struct Table { Trigger *pTrigger; /* List of triggers stored in pSchema */ Schema *pSchema; /* Schema that contains this table */ Table *pNextZombie; /* Next on the Parse.pZombieTab list */ + /** Space definition with Tarantool metadata. */ + struct space_def *def; }; /* diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 83c05ab48..274305352 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -35,6 +35,8 @@ */ #include "sqliteInt.h" #include "box/session.h" +#include "tarantoolInt.h" +#include "box/schema.h" /* * The most recently coded instruction was an OP_Column to retrieve the @@ -75,7 +77,14 @@ sqlite3ColumnDefault(Vdbe * v, Table * pTab, int i, int iReg) Column *pCol = &pTab->aCol[i]; VdbeComment((v, "%s.%s", pTab->zName, pCol->zName)); assert(i < pTab->nCol); - sqlite3ValueFromExpr(sqlite3VdbeDb(v), pCol->pDflt, + + Expr *expr = NULL; + struct space *space = + space_cache_find(SQLITE_PAGENO_TO_SPACEID(pTab->tnum)); + if (space != NULL && space->def->fields != NULL) + expr = space->def->fields[i].default_value_expr; + sqlite3ValueFromExpr(sqlite3VdbeDb(v), + expr, pCol->affinity, &pValue); if (pValue) { sqlite3VdbeAppendP4(v, pValue, P4_MEM);