* [tarantool-patches] [PATCH] sql: refactor primary index creation @ 2018-07-20 8:33 Kirill Yukhin 2018-07-20 12:11 ` [tarantool-patches] " n.pettik 0 siblings, 1 reply; 5+ messages in thread From: Kirill Yukhin @ 2018-07-20 8:33 UTC (permalink / raw) To: korablev; +Cc: tarantool-patches, Kirill Yukhin During tnum removal, convertToWithoutRowidtable became redundant. Remove it, refactor addPrimaryIndex. Also fix naming for un-named indexes. Finally, remove iPKey from struct table and update testsuite. Part of #3482 --- Issue: https://github.com/tarantool/tarantool/issues/3482 Branch: https://github.com/tarantool/tarantool/commits/kyukhin/gh-3482-remove-ipkey src/box/sql/build.c | 141 +++++++++++------------------ src/box/sql/fkey.c | 41 +-------- src/box/sql/insert.c | 49 +--------- src/box/sql/pragma.c | 30 +++--- src/box/sql/resolve.c | 17 +--- src/box/sql/select.c | 5 - src/box/sql/sqliteInt.h | 1 - src/box/sql/update.c | 40 ++++---- src/box/sql/where.c | 2 - test/sql-tap/analyze6.test.lua | 6 +- test/sql-tap/gh-2931-savepoints.test.lua | 2 +- test/sql-tap/gh2140-trans.test.lua | 2 +- test/sql-tap/gh2259-in-stmt-trans.test.lua | 8 +- test/sql-tap/gh2964-abort.test.lua | 2 +- test/sql-tap/index1.test.lua | 8 +- test/sql-tap/index7.test.lua | 8 +- test/sql-tap/intpkey.test.lua | 4 +- test/sql-tap/misc1.test.lua | 2 +- test/sql-tap/unique.test.lua | 8 +- test/sql-tap/update.test.lua | 6 +- test/sql/insert-unique.result | 2 +- test/sql/iproto.result | 2 +- test/sql/on-conflict.result | 2 +- test/sql/persistency.result | 4 +- test/sql/transition.result | 4 +- 25 files changed, 136 insertions(+), 260 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index a64d723..fc6681a 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -459,7 +459,6 @@ sql_table_new(Parse *parser, char *name) strcpy(table->def->engine_name, sql_storage_engine_strs[current_session()->sql_default_engine]); - table->iPKey = -1; table->iAutoIncPKey = -1; table->pSchema = db->pSchema; sqlite3HashInit(&table->idxHash); @@ -862,10 +861,6 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan) * a primary key (and this is the second primary key) then create an * error. * - * Set the Table.iPKey field of the table under construction to be the - * index of the INTEGER PRIMARY KEY column. - * Table.iPKey is set to -1 if there is no INTEGER PRIMARY KEY. - * * If the key is not an INTEGER PRIMARY KEY, then create a unique * index for the key. No index is created for INTEGER PRIMARY KEYs. */ @@ -923,7 +918,6 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ (pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) && sortOrder != SORT_ORDER_DESC) { assert(autoInc == 0 || autoInc == 1); - pTab->iPKey = iCol; pTab->keyConf = (u8) onError; if (autoInc) { pTab->iAutoIncPKey = iCol; @@ -931,6 +925,22 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ } if (pList) pParse->iPkSortOrder = pList->a[0].sort_order; + + struct sqlite3 *db = pParse->db; + struct ExprList *list; + struct Token ipkToken; + sqlite3TokenInit(&ipkToken, pTab->def->fields[iCol].name); + list = sql_expr_list_append(db, NULL, + sqlite3ExprAlloc(db, TK_ID, + &ipkToken, 0)); + if (list == NULL) + return; + list->a[0].sort_order = pParse->iPkSortOrder; + sql_create_index(pParse, 0, 0, list, pTab->keyConf, 0, 0, + SORT_ORDER_ASC, false, + SQL_INDEX_TYPE_CONSTRAINT_PK); + if (db->mallocFailed) + return; } else if (autoInc) { sqlite3ErrorMsg(pParse, "AUTOINCREMENT is only allowed on an " "INTEGER PRIMARY KEY or INT PRIMARY KEY"); @@ -941,7 +951,16 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ pList = 0; } - primary_key_exit: + /* Mark every PRIMARY KEY column as NOT NULL (except for imposter tables) + */ + for (uint32_t i = 0; i < pTab->def->field_count; i++) { + if (pTab->aCol[i].is_primkey) { + pTab->def->fields[i].nullable_action + = ON_CONFLICT_ACTION_ABORT; + pTab->def->fields[i].is_nullable = false; + } + } +primary_key_exit: sql_expr_list_delete(pParse->db, pList); return; } @@ -1215,63 +1234,6 @@ createTableStmt(sqlite3 * db, Table * p) return zStmt; } -/* - * This routine runs at the end of parsing a CREATE TABLE statement. - * The job of this routine is to convert both - * internal schema data structures and the generated VDBE code. - * Changes include: - * - * (1) Set all columns of the PRIMARY KEY schema object to be NOT NULL. - * (2) Set the Index.tnum of the PRIMARY KEY Index object in the - * schema to the rootpage from the main table. - * (3) Add all table columns to the PRIMARY KEY Index object - * so that the PRIMARY KEY is a covering index. - */ -static void -convertToWithoutRowidTable(Parse * pParse, Table * pTab) -{ - Index *pPk; - sqlite3 *db = pParse->db; - - /* Mark every PRIMARY KEY column as NOT NULL (except for imposter tables) - */ - if (!db->init.imposterTable) { - for (uint32_t i = 0; i < pTab->def->field_count; i++) { - if (pTab->aCol[i].is_primkey) { - pTab->def->fields[i].nullable_action - = ON_CONFLICT_ACTION_ABORT; - pTab->def->fields[i].is_nullable = false; - } - } - } - - /* Locate the PRIMARY KEY index. Or, if this table was originally - * an INTEGER PRIMARY KEY table, create a new PRIMARY KEY index. - */ - if (pTab->iPKey >= 0) { - ExprList *pList; - Token ipkToken; - sqlite3TokenInit(&ipkToken, pTab->def->fields[pTab->iPKey].name); - pList = sql_expr_list_append(pParse->db, NULL, - sqlite3ExprAlloc(db, TK_ID, - &ipkToken, 0)); - if (pList == 0) - return; - pList->a[0].sort_order = pParse->iPkSortOrder; - assert(pParse->pNewTable == pTab); - sql_create_index(pParse, 0, 0, pList, pTab->keyConf, 0, 0, - SORT_ORDER_ASC, false, - SQL_INDEX_TYPE_CONSTRAINT_PK); - if (db->mallocFailed) - return; - pPk = sqlite3PrimaryKeyIndex(pTab); - pTab->iPKey = -1; - } else { - pPk = sqlite3PrimaryKeyIndex(pTab); - } - assert(pPk != 0); -} - /* * Generate code to determine the new space Id. * Fetch the max space id seen so far from _schema and increment it. @@ -1652,8 +1614,6 @@ sqlite3EndTable(Parse * pParse, /* Parse context */ "PRIMARY KEY missing on table %s", p->def->name); return; - } else { - convertToWithoutRowidTable(pParse, p); } } @@ -2559,7 +2519,9 @@ tnt_error: static bool constraint_is_named(const char *name) { - return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")); + return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")) && + strncmp(name, "pk_unnamed_", strlen("pk_unnamed_")) && + strncmp(name, "unique_unnamed_", strlen("unique_unnamed_")); } void @@ -2648,29 +2610,36 @@ sql_create_index(struct Parse *parse, struct Token *token, sqlite3NameFromToken(db, &parse->constraintName); + /* + * This naming is temporary. Now it's not + * possible (since we implement UNIQUE + * and PK constraints with indexes and + * indexes can not have same names), but + * in future we would use names exactly + * as they are set by user. + */ + const char *prefix = "sql_autoindex_%s_%d"; + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) { + prefix = constraint_name == NULL ? + "unique_unnamed_%s_%d" : "unique_%s_%d"; + } + else { + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) + prefix = constraint_name == NULL ? + "pk_unnamed_%s_%d" : "pk_%s_%d"; + } + + uint32_t n = 1; + for (struct Index *idx = table->pIndex; idx != NULL; + idx = idx->pNext, n++); + if (constraint_name == NULL || strcmp(constraint_name, "") == 0) { - uint32_t n = 1; - for (struct Index *idx = table->pIndex; idx != NULL; - idx = idx->pNext, n++); - name = sqlite3MPrintf(db, "sql_autoindex_%s_%d", + name = sqlite3MPrintf(db, prefix, table->def->name, n); } else { - /* - * This naming is temporary. Now it's not - * possible (since we implement UNIQUE - * and PK constraints with indexes and - * indexes can not have same names), but - * in future we would use names exactly - * as they are set by user. - */ - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) - name = sqlite3MPrintf(db, - "unique_constraint_%s", - constraint_name); - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) - name = sqlite3MPrintf(db, "pk_constraint_%s", - constraint_name); + name = sqlite3MPrintf(db, prefix, + constraint_name, n); } sqlite3DbFree(db, constraint_name); } diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c index 6a91890..8cacfe7 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -230,23 +230,7 @@ sqlite3FkLocateIndex(Parse * pParse, /* Parse context to store any error in */ * space for the aiCol array (returned via output parameter *paiCol). * Non-composite foreign keys do not require the aiCol array. */ - if (nCol == 1) { - /* The FK maps to the IPK if any of the following are true: - * - * 1) There is an INTEGER PRIMARY KEY column and the FK is implicitly - * mapped to the primary key of table pParent, or - * 2) The FK is explicitly mapped to a column declared as INTEGER - * PRIMARY KEY. - */ - if (pParent->iPKey >= 0) { - if (!zKey) - return 0; - if (!strcmp(pParent->def->fields[pParent->iPKey].name, - zKey)) - return 0; - } - } else if (paiCol) { - assert(nCol > 1); + if (paiCol && nCol > 1) { aiCol = (int *)sqlite3DbMallocRawNN(pParse->db, nCol * sizeof(int)); if (!aiCol) @@ -470,11 +454,6 @@ fkLookupParent(Parse * pParse, /* Parse context */ int iChild = aiCol[i] + 1 + regData; int iParent = 1 + regData + (int)part->fieldno; - assert(aiCol[i] != pTab->iPKey); - if ((int)part->fieldno == pTab->iPKey) { - /* The parent key is a composite key that includes the IPK column */ - iParent = regData; - } sqlite3VdbeAddOp3(v, OP_Ne, iChild, iJump, iParent); VdbeCoverage(v); @@ -538,7 +517,7 @@ exprTableRegister(Parse * pParse, /* Parsing and code generating context */ pExpr = sqlite3Expr(db, TK_REGISTER, 0); if (pExpr) { - if (iCol >= 0 && iCol != pTab->iPKey) { + if (iCol >= 0) { pExpr->iTable = regBase + iCol + 1; char affinity = pTab->def->fields[iCol].affinity; pExpr->affinity = affinity; @@ -982,11 +961,6 @@ sqlite3FkCheck(Parse * pParse, /* Parse context */ iCol = pFKey->aCol[0].iFrom; aiCol = &iCol; } - for (i = 0; i < pFKey->nCol; i++) { - if (aiCol[i] == pTab->iPKey) { - aiCol[i] = -1; - } - } pParse->nTab++; @@ -1262,14 +1236,9 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, iFromCol = aiCol ? aiCol[i] : pFKey->aCol[0].iFrom; assert(iFromCol >= 0); - assert(pIdx != 0 - || (pTab->iPKey >= 0 - && pTab->iPKey < - (int)pTab->def->field_count)); - - uint32_t fieldno = pIdx != NULL ? - pIdx->def->key_def->parts[i].fieldno : - (uint32_t)pTab->iPKey; + assert(pIdx != NULL); + + uint32_t fieldno = pIdx->def->key_def->parts[i].fieldno; sqlite3TokenInit(&tToCol, pTab->def->fields[fieldno].name); sqlite3TokenInit(&tFromCol, diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 75bd6bd..3af9f9a 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -334,7 +334,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ int nColumn; /* Number of columns in the data */ int iDataCur = 0; /* VDBE cursor that is the main data repository */ int iIdxCur = 0; /* First index cursor */ - int ipkColumn = -1; /* Column that is the INTEGER PRIMARY KEY */ int endOfLoop; /* Label for the end of the insertion loop */ int srcTab = 0; /* Data comes from this temporary cursor if >=0 */ int addrInsTop = 0; /* Jump to label "D" */ @@ -444,13 +443,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ /* If the INSERT statement included an IDLIST term, then make sure * all elements of the IDLIST really are columns of the table and * remember the column indices. - * - * If the table has an INTEGER PRIMARY KEY column and that column - * is named in the IDLIST, then record in the ipkColumn variable - * the index into IDLIST of the primary key column. ipkColumn is - * the index of the primary key as it appears in IDLIST, not as - * is appears in the original table. (The index of the INTEGER - * PRIMARY KEY in the original table is pTab->iPKey.) */ /* Create bitmask to mark used columns of the table. */ void *used_columns = tt_static_buf(); @@ -470,10 +462,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ pColumn->a[i].idx = j; if (i != j) bIdListInOrder = 0; - if (j == pTab->iPKey) { - ipkColumn = i; - assert(is_view); - } break; } } @@ -603,14 +591,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ } } - /* If there is no IDLIST term but the table has an integer primary - * key, the set the ipkColumn variable to the integer primary key - * column index in the original table definition. - */ - if (pColumn == 0 && nColumn > 0) { - ipkColumn = pTab->iPKey; - } - if (pColumn == 0 && nColumn && nColumn != (int)def->field_count) { sqlite3ErrorMsg(pParse, "table %S has %d columns but %d values were supplied", @@ -742,18 +722,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */ * registers beginning at regIns. */ if (!is_view) { - if (ipkColumn >= 0) { - if (useTempTable) { - sqlite3VdbeAddOp3(v, OP_Column, srcTab, - ipkColumn, regTupleid); - } else if (pSelect) { - sqlite3VdbeAddOp2(v, OP_Copy, - regFromSelect + ipkColumn, - regTupleid); - } - } else { - sqlite3VdbeAddOp2(v, OP_Null, 0, regTupleid); - } + sqlite3VdbeAddOp2(v, OP_Null, 0, regTupleid); /* Compute data for all columns of the new entry, beginning * with the first column. @@ -866,7 +835,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */ on_conflict.optimized_action = ON_CONFLICT_ACTION_NONE; sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur, iIdxCur, regIns, 0, - ipkColumn >= 0, &on_conflict, + true, &on_conflict, endOfLoop, &isReplace, 0); sqlite3FkCheck(pParse, pTab, 0, regIns, 0); vdbe_emit_insertion_completion(v, iIdxCur, aRegIdx[0], @@ -1089,8 +1058,6 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ /* Test all NOT NULL constraints. */ for (uint32_t i = 0; i < def->field_count; i++) { - if ((int) i == pTab->iPKey) - continue; if (aiChng && aiChng[i] < 0) { /* Don't bother checking for NOT NULL on columns that do not change */ continue; @@ -1259,10 +1226,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ * needed for proper UNIQUE * constraint handling. */ - if ((int) fieldno == pTab->iPKey) - reg = regNewData; - else - reg = fieldno + regNewData + 1; + reg = fieldno + regNewData + 1; sqlite3VdbeAddOp2(v, OP_SCopy, reg, regIdx + i); VdbeComment((v, "%s", def->fields[fieldno].name)); @@ -1738,8 +1702,6 @@ xferOptimization(Parse * pParse, /* Parser context */ if (space_trigger_list(pDest->def->id) != NULL) return 0; if (onError == ON_CONFLICT_ACTION_DEFAULT) { - if (pDest->iPKey >= 0) - onError = pDest->keyConf; if (onError == ON_CONFLICT_ACTION_DEFAULT) onError = ON_CONFLICT_ACTION_ABORT; } @@ -1800,9 +1762,6 @@ xferOptimization(Parse * pParse, /* Parser context */ /* Number of columns must be the same in src and dst. */ if (pDest->def->field_count != pSrc->def->field_count) return 0; - /* Both tables must have the same INTEGER PRIMARY KEY. */ - if (pDest->iPKey != pSrc->iPKey) - return 0; for (i = 0; i < (int)pDest->def->field_count; i++) { enum affinity_type dest_affinity = pDest->def->fields[i].affinity; @@ -1893,7 +1852,7 @@ xferOptimization(Parse * pParse, /* Parser context */ regTupleid = sqlite3GetTempReg(pParse); sqlite3OpenTable(pParse, iDest, pDest, OP_OpenWrite); assert(destHasUniqueIdx); - if ((pDest->iPKey < 0 && pDest->pIndex != 0) /* (1) */ + if ((pDest->pIndex != 0) /* (1) */ ||destHasUniqueIdx /* (2) */ || (onError != ON_CONFLICT_ACTION_ABORT && onError != ON_CONFLICT_ACTION_ROLLBACK) /* (3) */ diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 7b1f6ec..cabe22b 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -726,22 +726,20 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ int iKey = pFK->aCol[0].iFrom; assert(iKey >= 0 && iKey < (int)pTab->def->field_count); - if (iKey != pTab->iPKey) { - sqlite3VdbeAddOp3(v, - OP_Column, - 0, - iKey, - regRow); - sqlite3ColumnDefault(v, - pTab->def, - iKey, - regRow); - sqlite3VdbeAddOp2(v, - OP_IsNull, - regRow, - addrOk); - VdbeCoverage(v); - } + sqlite3VdbeAddOp3(v, + OP_Column, + 0, + iKey, + regRow); + sqlite3ColumnDefault(v, + pTab->def, + iKey, + regRow); + sqlite3VdbeAddOp2(v, + OP_IsNull, + regRow, + addrOk); + VdbeCoverage(v); VdbeCoverage(v); sqlite3VdbeGoto(v, addrOk); sqlite3VdbeJumpHere(v, diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index a185473..43b2198 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -336,9 +336,6 @@ lookupName(Parse * pParse, /* The parsing context */ iCol++, pCol++) { if (strcmp(pTab->def->fields[iCol].name, zCol) == 0) { - if (iCol == pTab->iPKey) { - iCol = -1; - } break; } } @@ -498,15 +495,11 @@ sqlite3CreateColumnExpr(sqlite3 * db, SrcList * pSrc, int iSrc, int iCol) struct SrcList_item *pItem = &pSrc->a[iSrc]; p->space_def = pItem->pTab->def; p->iTable = pItem->iCursor; - if (pItem->pTab->iPKey == iCol) { - p->iColumn = -1; - } else { - p->iColumn = (ynVar) iCol; - testcase(iCol == BMS); - testcase(iCol == BMS - 1); - pItem->colUsed |= - ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol); - } + p->iColumn = (ynVar) iCol; + testcase(iCol == BMS); + testcase(iCol == BMS - 1); + pItem->colUsed |= + ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol); ExprSetProperty(p, EP_Resolved); } return p; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 34d5329..e548021 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1762,8 +1762,6 @@ generateColumnNames(Parse * pParse, /* Parser context */ } assert(j < pTabList->nSrc); pTab = pTabList->a[j].pTab; - if (iCol < 0) - iCol = pTab->iPKey; assert(iCol >= 0 && iCol < (int)pTab->def->field_count); zCol = pTab->def->fields[iCol].name; if (!shortNames && !fullNames) { @@ -2016,7 +2014,6 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * pSelect) assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); sqlite3ColumnsFromExprList(pParse, pSelect->pEList, table); sqlite3SelectAddColumnTypeAndCollation(pParse, table, pSelect); - table->iPKey = -1; if (db->mallocFailed) { sqlite3DeleteTable(db, table); return 0; @@ -4618,7 +4615,6 @@ withExpand(Walker * pWalker, struct SrcList_item *pFrom) if (pTab == NULL) return WRC_Abort; pTab->nTabRef = 1; - pTab->iPKey = -1; pTab->tuple_log_count = DEFAULT_TUPLE_LOG_COUNT; assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); @@ -4821,7 +4817,6 @@ selectExpander(Walker * pWalker, Select * p) sqlite3ColumnsFromExprList(pParse, pSel->pEList, pTab); if (sql_table_def_rebuild(db, pTab) != 0) return WRC_Abort; - pTab->iPKey = -1; pTab->tuple_log_count = DEFAULT_TUPLE_LOG_COUNT; assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index a205b72..f122b3f 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1918,7 +1918,6 @@ struct Table { Hash idxHash; /* All (named) indices indexed by name */ int tnum; /* Root BTree page for this table */ u32 nTabRef; /* Number of pointers to this Table */ - i16 iPKey; /* If not negative, use aCol[iPKey] as the rowid */ i16 iAutoIncPKey; /* If PK is marked INTEGER PRIMARY KEY AUTOINCREMENT, store column number here, -1 otherwise Tarantool specifics */ /** diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 167f425..d51a05c 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -468,28 +468,24 @@ sqlite3Update(Parse * pParse, /* The parser context */ newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1, TRIGGER_BEFORE, pTab, on_error); for (i = 0; i < (int)def->field_count; i++) { - if (i == pTab->iPKey) { - sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); + j = aXRef[i]; + if (j >= 0) { + sqlite3ExprCode(pParse, pChanges->a[j].pExpr, + regNew + i); + } else if (0 == (tmask & TRIGGER_BEFORE) || i > 31 + || (newmask & MASKBIT32(i))) { + /* This branch loads the value of a column that will not be changed + * into a register. This is done if there are no BEFORE triggers, or + * if there are one or more BEFORE triggers that use this value via + * a new.* reference in a trigger program. + */ + testcase(i == 31); + testcase(i == 32); + sqlite3ExprCodeGetColumnToReg(pParse, def, i, + iDataCur, + regNew + i); } else { - j = aXRef[i]; - if (j >= 0) { - sqlite3ExprCode(pParse, pChanges->a[j].pExpr, - regNew + i); - } else if (0 == (tmask & TRIGGER_BEFORE) || i > 31 - || (newmask & MASKBIT32(i))) { - /* This branch loads the value of a column that will not be changed - * into a register. This is done if there are no BEFORE triggers, or - * if there are one or more BEFORE triggers that use this value via - * a new.* reference in a trigger program. - */ - testcase(i == 31); - testcase(i == 32); - sqlite3ExprCodeGetColumnToReg(pParse, def, i, - iDataCur, - regNew + i); - } else { - sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); - } + sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); } } @@ -524,7 +520,7 @@ sqlite3Update(Parse * pParse, /* The parser context */ * registers in case this has happened. */ for (i = 0; i < (int)def->field_count; i++) { - if (aXRef[i] < 0 && i != pTab->iPKey) { + if (aXRef[i] < 0) { sqlite3ExprCodeGetColumnOfTable(v, def, iDataCur, i, regNew + i); diff --git a/src/box/sql/where.c b/src/box/sql/where.c index a4ba629..c115c4a 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -3478,8 +3478,6 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo, /* The WHERE clause */ pIndex->def->key_def; iColumn = def->parts[j].fieldno; revIdx = def->parts[j].sort_order; - if (iColumn == pIndex->pTable->iPKey) - iColumn = -1; } else { iColumn = -1; revIdx = 0; diff --git a/test/sql-tap/analyze6.test.lua b/test/sql-tap/analyze6.test.lua index cb1710a..4bbb67c 100755 --- a/test/sql-tap/analyze6.test.lua +++ b/test/sql-tap/analyze6.test.lua @@ -116,7 +116,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.2> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.2> }) @@ -148,7 +148,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.5> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.5> }) @@ -183,7 +183,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.8> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.8> }) diff --git a/test/sql-tap/gh-2931-savepoints.test.lua b/test/sql-tap/gh-2931-savepoints.test.lua index 3861bb2..11b4ac5 100755 --- a/test/sql-tap/gh-2931-savepoints.test.lua +++ b/test/sql-tap/gh-2931-savepoints.test.lua @@ -80,7 +80,7 @@ local testcases = { {0,{1,2,10,11,1,2,4,10,11}}}, {"14", [[insert into t1 values(4);]], - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}}, + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}}, {"15", [[select * from t1 union all select * from t2;]], {0,{1,2,10,11,1,2,4,10,11}}}, diff --git a/test/sql-tap/gh2140-trans.test.lua b/test/sql-tap/gh2140-trans.test.lua index fe7af5f..005163a 100755 --- a/test/sql-tap/gh2140-trans.test.lua +++ b/test/sql-tap/gh2140-trans.test.lua @@ -32,7 +32,7 @@ for _, verb in ipairs({'ROLLBACK', 'ABORT'}) do if verb == 'ROLLBACK' then answer = 'UNIQUE constraint failed: T1.S1' else - answer = "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + answer = "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" end test:do_catchsql_test('insert1_'..verb, [[BEGIN; diff --git a/test/sql-tap/gh2259-in-stmt-trans.test.lua b/test/sql-tap/gh2259-in-stmt-trans.test.lua index e2ae169..d1ced19 100755 --- a/test/sql-tap/gh2259-in-stmt-trans.test.lua +++ b/test/sql-tap/gh2259-in-stmt-trans.test.lua @@ -18,7 +18,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'_insert1', 'INSERT INTO t1 VALUES(1, 2)', - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test(prefix..'_insert1_check1', 'SELECT * FROM t1', @@ -34,7 +34,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'_update1', 'UPDATE t1 SET s1=1', - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test(prefix..'_update1_check1', 'SELECT * FROM t1', @@ -52,7 +52,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'delete1', 'DELETE FROM t1;', - {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) -- Nothing should be inserted due to abort test:do_execsql_test('delete1_check1', @@ -69,7 +69,7 @@ end -- Check multi-insert test:do_catchsql_test('insert2', 'INSERT INTO t1 VALUES (5, 6), (6, 7)', - {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test('insert2_check', 'SELECT * FROM t1;', {3, 3}) diff --git a/test/sql-tap/gh2964-abort.test.lua b/test/sql-tap/gh2964-abort.test.lua index a06b4fd..52809d8 100755 --- a/test/sql-tap/gh2964-abort.test.lua +++ b/test/sql-tap/gh2964-abort.test.lua @@ -13,7 +13,7 @@ test:do_catchsql_test( "CREATE TABLE t2 (a int primary key);") local insert_err = {1, "UNIQUE constraint failed: T2.A"} -local insert_err_PK = {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"} +local insert_err_PK = {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"} local data = { --id|TRIG TYPE|INSERT TYPE|insert error|commit error| result {1, "AFTER", "or abort", insert_err_PK, {0}, {1,1,2}}, diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua index 9ee40fa..78205f8 100755 --- a/test/sql-tap/index1.test.lua +++ b/test/sql-tap/index1.test.lua @@ -444,10 +444,10 @@ test:do_execsql_test( test:do_execsql_test( "index-7.3", [[ - SELECT "name" FROM "_index" WHERE "name"='sql_autoindex_TEST1_1' + SELECT "name" FROM "_index" WHERE "name"='pk_unnamed_TEST1_1' ]], { -- <index-7.3> - "sql_autoindex_TEST1_1" + "pk_unnamed_TEST1_1" -- </index-7.3> }) @@ -1017,7 +1017,7 @@ test:do_execsql_test( SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE "_index"."id" = "_space"."id" AND "_space"."name"='T7'; ]], { -- <index-17.1> - "sql_autoindex_T7_3", "sql_autoindex_T7_2", "sql_autoindex_T7_1" + "pk_unnamed_T7_3", "unique_unnamed_T7_2", "unique_unnamed_T7_1" -- </index-17.1> }) @@ -1071,7 +1071,7 @@ test:do_execsql_test( INSERT INTO t7 VALUES(1); ]], { -- <index-19.2> - 1, "Duplicate key exists in unique index 'sql_autoindex_T7_1' in space 'T7'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T7_1' in space 'T7'" -- </index-19.2> }) diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua index 3914e3c..4f90708 100755 --- a/test/sql-tap/index7.test.lua +++ b/test/sql-tap/index7.test.lua @@ -355,7 +355,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST5'; ]], - {0, {"sql_autoindex_TEST5_1",0}}) + {0, {"pk_unnamed_TEST5_1",0}}) -- This test checks that CREATE TABLE statement with PK constraint -- and NAMED UNIQUE constraint (declared on THE SAME COLUMNS) @@ -371,7 +371,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST6'; ]], - {0, {"sql_autoindex_TEST6_1",0,"unique_constraint_C1",1}}) + {0, {"pk_unnamed_TEST6_1",0,"unique_C1_2",1}}) -- This test checks that CREATE TABLE statement with PK constraint -- and UNIQUE constraint is executed correctly @@ -386,7 +386,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST7'; ]], - {0, {"sql_autoindex_TEST7_1",0}}) + {0, {"unique_unnamed_TEST7_1",0}}) -- This test is the same as previous, but with named UNIQUE @@ -401,6 +401,6 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST8'; ]], - {0, {"sql_autoindex_TEST8_2",0,"unique_constraint_C1",1}}) + {0, {"pk_TEST8_2",0,"unique_C1_1",1}}) test:finish_test() diff --git a/test/sql-tap/intpkey.test.lua b/test/sql-tap/intpkey.test.lua index b5359b6..3193e48 100755 --- a/test/sql-tap/intpkey.test.lua +++ b/test/sql-tap/intpkey.test.lua @@ -42,7 +42,7 @@ test:do_execsql_test( SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE "_index"."id" = "_space"."id" AND "_space"."name"='T1' ]], { -- <intpkey-1.1> - "sql_autoindex_T1_1" + "pk_unnamed_T1_1" -- </intpkey-1.1> }) @@ -96,7 +96,7 @@ test:do_catchsql_test( INSERT INTO t1 VALUES(5,'second','entry'); ]], { -- <intpkey-1.6> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </intpkey-1.6> }) diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index a5078b5..0fae8d1 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -380,7 +380,7 @@ test:do_catchsql_test( INSERT INTO t5 VALUES(1,2,4); ]], { -- <misc1-7.4> - 1, "Duplicate key exists in unique index 'sql_autoindex_T5_1' in space 'T5'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T5_1' in space 'T5'" -- </misc1-7.4> }) diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua index 63b5065..3856d26 100755 --- a/test/sql-tap/unique.test.lua +++ b/test/sql-tap/unique.test.lua @@ -70,7 +70,7 @@ test:do_catchsql_test( INSERT INTO t1(a,b,c) VALUES(1,3,4) ]], { -- <unique-1.3> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_2' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </unique-1.3> }) @@ -91,7 +91,7 @@ test:do_catchsql_test( INSERT INTO t1(a,b,c) VALUES(3,2,4) ]], { -- <unique-1.5> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'" -- </unique-1.5> }) @@ -287,7 +287,7 @@ test:do_catchsql_test( SELECT a,b,c,d FROM t3 ORDER BY a,b,c,d; ]], { -- <unique-3.4> - 1, "Duplicate key exists in unique index 'sql_autoindex_T3_2' in space 'T3'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T3_2' in space 'T3'" -- </unique-3.4> }) @@ -444,7 +444,7 @@ test:do_catchsql_test( INSERT INTO t5 VALUES(2, 1,2,3,4,5,6); ]], { -- <unique-5.2> - 1, "Duplicate key exists in unique index 'sql_autoindex_T5_2' in space 'T5'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T5_2' in space 'T5'" -- </unique-5.2> }) diff --git a/test/sql-tap/update.test.lua b/test/sql-tap/update.test.lua index 1ed951d..d4debc7 100755 --- a/test/sql-tap/update.test.lua +++ b/test/sql-tap/update.test.lua @@ -917,7 +917,7 @@ test:do_catchsql_test("update-10.3", [[ SELECT * FROM t1; ]], { -- <update-10.3> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_3' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </update-10.3> }) @@ -943,7 +943,7 @@ test:do_catchsql_test("update-10.6", [[ SELECT * FROM t1; ]], { -- <update-10.6> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'" -- </update-10.6> }) @@ -969,7 +969,7 @@ test:do_catchsql_test("update-10.9", [[ SELECT * FROM t1; ]], { -- <update-10.9> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_2' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_3' in space 'T1'" -- </update-10.9> }) diff --git a/test/sql/insert-unique.result b/test/sql/insert-unique.result index 359ac43..797c8ef 100644 --- a/test/sql/insert-unique.result +++ b/test/sql/insert-unique.result @@ -24,7 +24,7 @@ box.sql.execute("INSERT INTO zoobar VALUES (111, 222, 'c3', 444)") -- PK must be unique box.sql.execute("INSERT INTO zoobar VALUES (112, 222, 'c3', 444)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_ZOOBAR_1' in space 'ZOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_ZOOBAR_1' in space 'ZOOBAR' ... -- Unique index must be respected box.sql.execute("INSERT INTO zoobar VALUES (111, 223, 'c3', 444)") diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 26ad17b..af474bc 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -553,7 +553,7 @@ future1:wait_result() future2:wait_result() --- - null -- 'Failed to execute SQL statement: Duplicate key exists in unique index ''sql_autoindex_TEST_1'' +- 'Failed to execute SQL statement: Duplicate key exists in unique index ''pk_unnamed_TEST_1'' in space ''TEST''' ... future3:wait_result() diff --git a/test/sql/on-conflict.result b/test/sql/on-conflict.result index 4080648..b4772d8 100644 --- a/test/sql/on-conflict.result +++ b/test/sql/on-conflict.result @@ -23,7 +23,7 @@ box.sql.execute("CREATE TABLE e (id INTEGER PRIMARY KEY ON CONFLICT REPLACE, v I -- Insert values and select them box.sql.execute("INSERT INTO t values (1, 1), (2, 2), (3, 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_T_1' in space 'T' +- error: Duplicate key exists in unique index 'unique_unnamed_T_2' in space 'T' ... box.sql.execute("SELECT * FROM t") --- diff --git a/test/sql/persistency.result b/test/sql/persistency.result index f64e666..c65baa0 100644 --- a/test/sql/persistency.result +++ b/test/sql/persistency.result @@ -26,7 +26,7 @@ box.sql.execute("INSERT INTO foobar VALUES (1000, 'foobar')") ... box.sql.execute("INSERT INTO foobar VALUES (1, 'duplicate')") --- -- error: Duplicate key exists in unique index 'sql_autoindex_FOOBAR_1' in space 'FOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_FOOBAR_1' in space 'FOOBAR' ... -- simple select box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar") @@ -208,7 +208,7 @@ box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"") -- prove barfoo2 still exists box.sql.execute("INSERT INTO barfoo VALUES ('xfoo', 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_BARFOO_1' in space 'BARFOO' +- error: Duplicate key exists in unique index 'pk_unnamed_BARFOO_1' in space 'BARFOO' ... box.sql.execute("SELECT * FROM barfoo") --- diff --git a/test/sql/transition.result b/test/sql/transition.result index 765b0f0..805e8aa 100644 --- a/test/sql/transition.result +++ b/test/sql/transition.result @@ -23,7 +23,7 @@ box.sql.execute("INSERT INTO foobar VALUES (1000, 'foobar')") ... box.sql.execute("INSERT INTO foobar VALUES (1, 'duplicate')") --- -- error: Duplicate key exists in unique index 'sql_autoindex_FOOBAR_1' in space 'FOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_FOOBAR_1' in space 'FOOBAR' ... -- simple select box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar") @@ -141,7 +141,7 @@ box.sql.execute("INSERT INTO barfoo VALUES ('foobar', 1000)") -- prove barfoo2 was created box.sql.execute("INSERT INTO barfoo VALUES ('xfoo', 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_BARFOO_1' in space 'BARFOO' +- error: Duplicate key exists in unique index 'pk_unnamed_BARFOO_1' in space 'BARFOO' ... box.sql.execute("SELECT foo, bar FROM barfoo") --- -- 2.16.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor primary index creation 2018-07-20 8:33 [tarantool-patches] [PATCH] sql: refactor primary index creation Kirill Yukhin @ 2018-07-20 12:11 ` n.pettik 2018-07-20 13:08 ` Kirill Yukhin 0 siblings, 1 reply; 5+ messages in thread From: n.pettik @ 2018-07-20 12:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Yukhin Now you also can remove keyConf field from struct Table. > @@ -931,6 +925,22 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ > } > if (pList) > pParse->iPkSortOrder = pList->a[0].sort_order; > + > + struct sqlite3 *db = pParse->db; > + struct ExprList *list; > + struct Token ipkToken; > + sqlite3TokenInit(&ipkToken, pTab->def->fields[iCol].name); > + list = sql_expr_list_append(db, NULL, > + sqlite3ExprAlloc(db, TK_ID, > + &ipkToken, 0)); > + if (list == NULL) > + return; You don’t need to create list from scratch: sql_create_index accepts NULL as list of columns and processes it exactly in the same way (i.e. uses only last column). > + list->a[0].sort_order = pParse->iPkSortOrder; > + sql_create_index(pParse, 0, 0, list, pTab->keyConf, 0, 0, > + SORT_ORDER_ASC, false, > + SQL_INDEX_TYPE_CONSTRAINT_PK); > + if (db->mallocFailed) > + return; goto primary_key_exit? > } else if (autoInc) { > sqlite3ErrorMsg(pParse, "AUTOINCREMENT is only allowed on an " > "INTEGER PRIMARY KEY or INT PRIMARY KEY"); > @@ -941,7 +951,16 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ > pList = 0; > } > > - primary_key_exit: > + /* Mark every PRIMARY KEY column as NOT NULL (except for imposter tables) > + */ Fix comment: we don’t have imposter tables; fit it into 66 chars. > + for (uint32_t i = 0; i < pTab->def->field_count; i++) { > + if (pTab->aCol[i].is_primkey) { > + pTab->def->fields[i].nullable_action > + = ON_CONFLICT_ACTION_ABORT; > + pTab->def->fields[i].is_nullable = false; > + } > + } > +primary_key_exit: > sql_expr_list_delete(pParse->db, pList); > return; > } > @@ -1215,63 +1234,6 @@ createTableStmt(sqlite3 * db, Table * p) > return zStmt; > } > > -/* > - * This routine runs at the end of parsing a CREATE TABLE statement. > - * The job of this routine is to convert both > - * internal schema data structures and the generated VDBE code. > - * Changes include: > - * > - * (1) Set all columns of the PRIMARY KEY schema object to be NOT NULL. > - * (2) Set the Index.tnum of the PRIMARY KEY Index object in the > - * schema to the rootpage from the main table. > - * (3) Add all table columns to the PRIMARY KEY Index object > - * so that the PRIMARY KEY is a covering index. > - */ > -static void > -convertToWithoutRowidTable(Parse * pParse, Table * pTab) I see reference to this function in comment to struct Index. Remove it as well. > > @@ -2559,7 +2519,9 @@ tnt_error: > static bool > constraint_is_named(const char *name) > { > - return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")); > + return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")) && > + strncmp(name, "pk_unnamed_", strlen("pk_unnamed_")) && > + strncmp(name, "unique_unnamed_", strlen("unique_unnamed_")); > } > > void > @@ -2648,29 +2610,36 @@ sql_create_index(struct Parse *parse, struct Token *token, > sqlite3NameFromToken(db, > &parse->constraintName); > > + /* > + * This naming is temporary. Now it's not > + * possible (since we implement UNIQUE > + * and PK constraints with indexes and > + * indexes can not have same names), but > + * in future we would use names exactly > + * as they are set by user. > + */ > + const char *prefix = "sql_autoindex_%s_%d”; If we trapped so far, index is definitely UNIQUE or PK constraint, so this initialisation with default prefix is redundant. And actually I don’t understand why do you need this renaming... > + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) { > + prefix = constraint_name == NULL ? > + "unique_unnamed_%s_%d" : "unique_%s_%d"; > + } > + else { > + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) > + prefix = constraint_name == NULL ? > + "pk_unnamed_%s_%d" : "pk_%s_%d"; > + } > + > + uint32_t n = 1; > + for (struct Index *idx = table->pIndex; idx != NULL; > + idx = idx->pNext, n++); > + > if (constraint_name == NULL || > strcmp(constraint_name, "") == 0) { > - uint32_t n = 1; > - for (struct Index *idx = table->pIndex; idx != NULL; > - idx = idx->pNext, n++); > - name = sqlite3MPrintf(db, "sql_autoindex_%s_%d", > + name = sqlite3MPrintf(db, prefix, > table->def->name, n); > } else { > - /* > - * This naming is temporary. Now it's not > - * possible (since we implement UNIQUE > - * and PK constraints with indexes and > - * indexes can not have same names), but > - * in future we would use names exactly > - * as they are set by user. > - */ > - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) > - name = sqlite3MPrintf(db, > - "unique_constraint_%s", > - constraint_name); > - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) > - name = sqlite3MPrintf(db, "pk_constraint_%s", > - constraint_name); > + name = sqlite3MPrintf(db, prefix, > + constraint_name, n); > } > sqlite3DbFree(db, constraint_name); > } > > diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c > index 7b1f6ec..cabe22b 100644 > --- a/src/box/sql/pragma.c > +++ b/src/box/sql/pragma.c > @@ -726,22 +726,20 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ > int iKey = pFK->aCol[0].iFrom; > assert(iKey >= 0 && iKey < > (int)pTab->def->field_count); > - if (iKey != pTab->iPKey) { > - sqlite3VdbeAddOp3(v, > - OP_Column, > - 0, > - iKey, > - regRow); > - sqlite3ColumnDefault(v, > - pTab->def, > - iKey, > - regRow); > - sqlite3VdbeAddOp2(v, > - OP_IsNull, > - regRow, > - addrOk); > - VdbeCoverage(v); > - } > + sqlite3VdbeAddOp3(v, > + OP_Column, > + 0, > + iKey, > + regRow); > + sqlite3ColumnDefault(v, > + pTab->def, > + iKey, > + regRow); > + sqlite3VdbeAddOp2(v, > + OP_IsNull, > + regRow, > + addrOk); > + VdbeCoverage(v); > VdbeCoverage(v); Double call of VdbeCoverage(); ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor primary index creation 2018-07-20 12:11 ` [tarantool-patches] " n.pettik @ 2018-07-20 13:08 ` Kirill Yukhin 2018-07-20 14:34 ` n.pettik 0 siblings, 1 reply; 5+ messages in thread From: Kirill Yukhin @ 2018-07-20 13:08 UTC (permalink / raw) To: n.pettik; +Cc: tarantool-patches Hello Nikita, Thanks for review. My answers inline. Updated patch in the bottom. On 20 июл 15:11, n.pettik wrote: > Now you also can remove keyConf field from struct Table. Done, thanks! > > @@ -931,6 +925,22 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ > > } > > if (pList) > > pParse->iPkSortOrder = pList->a[0].sort_order; > > + > > + struct sqlite3 *db = pParse->db; > > + struct ExprList *list; > > + struct Token ipkToken; > > + sqlite3TokenInit(&ipkToken, pTab->def->fields[iCol].name); > > + list = sql_expr_list_append(db, NULL, > > + sqlite3ExprAlloc(db, TK_ID, > > + &ipkToken, 0)); > > + if (list == NULL) > > + return; > You don’t need to create list from scratch: sql_create_index accepts > NULL as list of columns and processes it exactly in the same way > (i.e. uses only last column). Yes it is, but in case of NULL it creates a list out of last column inserted, while this code creates a list out of iCol column. So it cannot be removed. A refactored it a bit: removed iPkSortOrder from struct Parse. > > + list->a[0].sort_order = pParse->iPkSortOrder; > > + sql_create_index(pParse, 0, 0, list, pTab->keyConf, 0, 0, > > + SORT_ORDER_ASC, false, > > + SQL_INDEX_TYPE_CONSTRAINT_PK); > > + if (db->mallocFailed) > > + return; > goto primary_key_exit? Fixed. > > @@ -941,7 +951,16 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ > > pList = 0; > > } > > > > - primary_key_exit: > > + /* Mark every PRIMARY KEY column as NOT NULL (except for imposter tables) > > + */ > Fix comment: we don’t have imposter tables; fit it into 66 chars. Fixed. > > -static void > > -convertToWithoutRowidTable(Parse * pParse, Table * pTab) > I see reference to this function in comment to struct Index. > Remove it as well. Done (altough it is removed in the next patch on tnum removal). > > void > > @@ -2648,29 +2610,36 @@ sql_create_index(struct Parse *parse, struct Token *token, > > sqlite3NameFromToken(db, > > &parse->constraintName); > > > > + /* > > + * This naming is temporary. Now it's not > > + * possible (since we implement UNIQUE > > + * and PK constraints with indexes and > > + * indexes can not have same names), but > > + * in future we would use names exactly > > + * as they are set by user. > > + */ > > + const char *prefix = "sql_autoindex_%s_%d”; > > If we trapped so far, index is definitely UNIQUE or PK constraint, > so this initialisation with default prefix is redundant. Fixed, assert added. > And actually I don’t understand why do you need this renaming... Since after converttowithoutrowid removal, index ids were twisted. So, its better to fix wrong names simultaneously w/ index is updates in test suite. > > diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c > > index 7b1f6ec..cabe22b 100644 > > --- a/src/box/sql/pragma.c > > +++ b/src/box/sql/pragma.c > > + VdbeCoverage(v); > > VdbeCoverage(v); > Double call of VdbeCoverage(); Fixed. -- Regards, Kirill Yukhin commit 21b1a0e4b78a4d6b999399aeabab8c04d2c76188 Author: Kirill Yukhin <kyukhin@tarantool.org> Date: Thu Jul 19 22:10:00 2018 +0300 sql: refactor primary index creation During tnum removal, convertToWithoutRowidtable became redundant. Remove it, refactor addPrimaryIndex. Also fix naming for un-named indexes. Finally, remove iPKey from struct table and update testsuite. Part of #3482 diff --git a/src/box/sql/build.c b/src/box/sql/build.c index a64d723..cb7f1e4 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -459,7 +459,6 @@ sql_table_new(Parse *parser, char *name) strcpy(table->def->engine_name, sql_storage_engine_strs[current_session()->sql_default_engine]); - table->iPKey = -1; table->iAutoIncPKey = -1; table->pSchema = db->pSchema; sqlite3HashInit(&table->idxHash); @@ -862,10 +861,6 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan) * a primary key (and this is the second primary key) then create an * error. * - * Set the Table.iPKey field of the table under construction to be the - * index of the INTEGER PRIMARY KEY column. - * Table.iPKey is set to -1 if there is no INTEGER PRIMARY KEY. - * * If the key is not an INTEGER PRIMARY KEY, then create a unique * index for the key. No index is created for INTEGER PRIMARY KEYs. */ @@ -923,14 +918,24 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ (pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) && sortOrder != SORT_ORDER_DESC) { assert(autoInc == 0 || autoInc == 1); - pTab->iPKey = iCol; - pTab->keyConf = (u8) onError; if (autoInc) { pTab->iAutoIncPKey = iCol; pTab->tabFlags |= TF_Autoincrement; } - if (pList) - pParse->iPkSortOrder = pList->a[0].sort_order; + struct sqlite3 *db = pParse->db; + struct ExprList *list; + struct Token token; + sqlite3TokenInit(&token, pTab->def->fields[iCol].name); + list = sql_expr_list_append(db, NULL, + sqlite3ExprAlloc(db, TK_ID, + &token, 0)); + if (list == NULL) + return; + sql_create_index(pParse, 0, 0, list, onError, 0, 0, + SORT_ORDER_ASC, false, + SQL_INDEX_TYPE_CONSTRAINT_PK); + if (db->mallocFailed) + goto primary_key_exit; } else if (autoInc) { sqlite3ErrorMsg(pParse, "AUTOINCREMENT is only allowed on an " "INTEGER PRIMARY KEY or INT PRIMARY KEY"); @@ -941,7 +946,15 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ pList = 0; } - primary_key_exit: + /* Mark every PRIMARY KEY column as NOT NULL. */ + for (uint32_t i = 0; i < pTab->def->field_count; i++) { + if (pTab->aCol[i].is_primkey) { + pTab->def->fields[i].nullable_action + = ON_CONFLICT_ACTION_ABORT; + pTab->def->fields[i].is_nullable = false; + } + } +primary_key_exit: sql_expr_list_delete(pParse->db, pList); return; } @@ -1215,63 +1228,6 @@ createTableStmt(sqlite3 * db, Table * p) return zStmt; } -/* - * This routine runs at the end of parsing a CREATE TABLE statement. - * The job of this routine is to convert both - * internal schema data structures and the generated VDBE code. - * Changes include: - * - * (1) Set all columns of the PRIMARY KEY schema object to be NOT NULL. - * (2) Set the Index.tnum of the PRIMARY KEY Index object in the - * schema to the rootpage from the main table. - * (3) Add all table columns to the PRIMARY KEY Index object - * so that the PRIMARY KEY is a covering index. - */ -static void -convertToWithoutRowidTable(Parse * pParse, Table * pTab) -{ - Index *pPk; - sqlite3 *db = pParse->db; - - /* Mark every PRIMARY KEY column as NOT NULL (except for imposter tables) - */ - if (!db->init.imposterTable) { - for (uint32_t i = 0; i < pTab->def->field_count; i++) { - if (pTab->aCol[i].is_primkey) { - pTab->def->fields[i].nullable_action - = ON_CONFLICT_ACTION_ABORT; - pTab->def->fields[i].is_nullable = false; - } - } - } - - /* Locate the PRIMARY KEY index. Or, if this table was originally - * an INTEGER PRIMARY KEY table, create a new PRIMARY KEY index. - */ - if (pTab->iPKey >= 0) { - ExprList *pList; - Token ipkToken; - sqlite3TokenInit(&ipkToken, pTab->def->fields[pTab->iPKey].name); - pList = sql_expr_list_append(pParse->db, NULL, - sqlite3ExprAlloc(db, TK_ID, - &ipkToken, 0)); - if (pList == 0) - return; - pList->a[0].sort_order = pParse->iPkSortOrder; - assert(pParse->pNewTable == pTab); - sql_create_index(pParse, 0, 0, pList, pTab->keyConf, 0, 0, - SORT_ORDER_ASC, false, - SQL_INDEX_TYPE_CONSTRAINT_PK); - if (db->mallocFailed) - return; - pPk = sqlite3PrimaryKeyIndex(pTab); - pTab->iPKey = -1; - } else { - pPk = sqlite3PrimaryKeyIndex(pTab); - } - assert(pPk != 0); -} - /* * Generate code to determine the new space Id. * Fetch the max space id seen so far from _schema and increment it. @@ -1652,8 +1608,6 @@ sqlite3EndTable(Parse * pParse, /* Parse context */ "PRIMARY KEY missing on table %s", p->def->name); return; - } else { - convertToWithoutRowidTable(pParse, p); } } @@ -2559,7 +2513,9 @@ tnt_error: static bool constraint_is_named(const char *name) { - return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")); + return strncmp(name, "sql_autoindex_", strlen("sql_autoindex_")) && + strncmp(name, "pk_unnamed_", strlen("pk_unnamed_")) && + strncmp(name, "unique_unnamed_", strlen("unique_unnamed_")); } void @@ -2648,29 +2604,37 @@ sql_create_index(struct Parse *parse, struct Token *token, sqlite3NameFromToken(db, &parse->constraintName); + /* + * This naming is temporary. Now it's not + * possible (since we implement UNIQUE + * and PK constraints with indexes and + * indexes can not have same names), but + * in future we would use names exactly + * as they are set by user. + */ + assert(idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE || + idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK); + const char *prefix = NULL; + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) { + prefix = constraint_name == NULL ? + "unique_unnamed_%s_%d" : "unique_%s_%d"; + } + else { /* idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK */ + prefix = constraint_name == NULL ? + "pk_unnamed_%s_%d" : "pk_%s_%d"; + } + + uint32_t n = 1; + for (struct Index *idx = table->pIndex; idx != NULL; + idx = idx->pNext, n++); + if (constraint_name == NULL || strcmp(constraint_name, "") == 0) { - uint32_t n = 1; - for (struct Index *idx = table->pIndex; idx != NULL; - idx = idx->pNext, n++); - name = sqlite3MPrintf(db, "sql_autoindex_%s_%d", + name = sqlite3MPrintf(db, prefix, table->def->name, n); } else { - /* - * This naming is temporary. Now it's not - * possible (since we implement UNIQUE - * and PK constraints with indexes and - * indexes can not have same names), but - * in future we would use names exactly - * as they are set by user. - */ - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) - name = sqlite3MPrintf(db, - "unique_constraint_%s", - constraint_name); - if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) - name = sqlite3MPrintf(db, "pk_constraint_%s", - constraint_name); + name = sqlite3MPrintf(db, prefix, + constraint_name, n); } sqlite3DbFree(db, constraint_name); } diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c index 6a91890..8cacfe7 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -230,23 +230,7 @@ sqlite3FkLocateIndex(Parse * pParse, /* Parse context to store any error in */ * space for the aiCol array (returned via output parameter *paiCol). * Non-composite foreign keys do not require the aiCol array. */ - if (nCol == 1) { - /* The FK maps to the IPK if any of the following are true: - * - * 1) There is an INTEGER PRIMARY KEY column and the FK is implicitly - * mapped to the primary key of table pParent, or - * 2) The FK is explicitly mapped to a column declared as INTEGER - * PRIMARY KEY. - */ - if (pParent->iPKey >= 0) { - if (!zKey) - return 0; - if (!strcmp(pParent->def->fields[pParent->iPKey].name, - zKey)) - return 0; - } - } else if (paiCol) { - assert(nCol > 1); + if (paiCol && nCol > 1) { aiCol = (int *)sqlite3DbMallocRawNN(pParse->db, nCol * sizeof(int)); if (!aiCol) @@ -470,11 +454,6 @@ fkLookupParent(Parse * pParse, /* Parse context */ int iChild = aiCol[i] + 1 + regData; int iParent = 1 + regData + (int)part->fieldno; - assert(aiCol[i] != pTab->iPKey); - if ((int)part->fieldno == pTab->iPKey) { - /* The parent key is a composite key that includes the IPK column */ - iParent = regData; - } sqlite3VdbeAddOp3(v, OP_Ne, iChild, iJump, iParent); VdbeCoverage(v); @@ -538,7 +517,7 @@ exprTableRegister(Parse * pParse, /* Parsing and code generating context */ pExpr = sqlite3Expr(db, TK_REGISTER, 0); if (pExpr) { - if (iCol >= 0 && iCol != pTab->iPKey) { + if (iCol >= 0) { pExpr->iTable = regBase + iCol + 1; char affinity = pTab->def->fields[iCol].affinity; pExpr->affinity = affinity; @@ -982,11 +961,6 @@ sqlite3FkCheck(Parse * pParse, /* Parse context */ iCol = pFKey->aCol[0].iFrom; aiCol = &iCol; } - for (i = 0; i < pFKey->nCol; i++) { - if (aiCol[i] == pTab->iPKey) { - aiCol[i] = -1; - } - } pParse->nTab++; @@ -1262,14 +1236,9 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, iFromCol = aiCol ? aiCol[i] : pFKey->aCol[0].iFrom; assert(iFromCol >= 0); - assert(pIdx != 0 - || (pTab->iPKey >= 0 - && pTab->iPKey < - (int)pTab->def->field_count)); - - uint32_t fieldno = pIdx != NULL ? - pIdx->def->key_def->parts[i].fieldno : - (uint32_t)pTab->iPKey; + assert(pIdx != NULL); + + uint32_t fieldno = pIdx->def->key_def->parts[i].fieldno; sqlite3TokenInit(&tToCol, pTab->def->fields[fieldno].name); sqlite3TokenInit(&tFromCol, diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 75bd6bd..3af9f9a 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -334,7 +334,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ int nColumn; /* Number of columns in the data */ int iDataCur = 0; /* VDBE cursor that is the main data repository */ int iIdxCur = 0; /* First index cursor */ - int ipkColumn = -1; /* Column that is the INTEGER PRIMARY KEY */ int endOfLoop; /* Label for the end of the insertion loop */ int srcTab = 0; /* Data comes from this temporary cursor if >=0 */ int addrInsTop = 0; /* Jump to label "D" */ @@ -444,13 +443,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ /* If the INSERT statement included an IDLIST term, then make sure * all elements of the IDLIST really are columns of the table and * remember the column indices. - * - * If the table has an INTEGER PRIMARY KEY column and that column - * is named in the IDLIST, then record in the ipkColumn variable - * the index into IDLIST of the primary key column. ipkColumn is - * the index of the primary key as it appears in IDLIST, not as - * is appears in the original table. (The index of the INTEGER - * PRIMARY KEY in the original table is pTab->iPKey.) */ /* Create bitmask to mark used columns of the table. */ void *used_columns = tt_static_buf(); @@ -470,10 +462,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ pColumn->a[i].idx = j; if (i != j) bIdListInOrder = 0; - if (j == pTab->iPKey) { - ipkColumn = i; - assert(is_view); - } break; } } @@ -603,14 +591,6 @@ sqlite3Insert(Parse * pParse, /* Parser context */ } } - /* If there is no IDLIST term but the table has an integer primary - * key, the set the ipkColumn variable to the integer primary key - * column index in the original table definition. - */ - if (pColumn == 0 && nColumn > 0) { - ipkColumn = pTab->iPKey; - } - if (pColumn == 0 && nColumn && nColumn != (int)def->field_count) { sqlite3ErrorMsg(pParse, "table %S has %d columns but %d values were supplied", @@ -742,18 +722,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */ * registers beginning at regIns. */ if (!is_view) { - if (ipkColumn >= 0) { - if (useTempTable) { - sqlite3VdbeAddOp3(v, OP_Column, srcTab, - ipkColumn, regTupleid); - } else if (pSelect) { - sqlite3VdbeAddOp2(v, OP_Copy, - regFromSelect + ipkColumn, - regTupleid); - } - } else { - sqlite3VdbeAddOp2(v, OP_Null, 0, regTupleid); - } + sqlite3VdbeAddOp2(v, OP_Null, 0, regTupleid); /* Compute data for all columns of the new entry, beginning * with the first column. @@ -866,7 +835,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */ on_conflict.optimized_action = ON_CONFLICT_ACTION_NONE; sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur, iIdxCur, regIns, 0, - ipkColumn >= 0, &on_conflict, + true, &on_conflict, endOfLoop, &isReplace, 0); sqlite3FkCheck(pParse, pTab, 0, regIns, 0); vdbe_emit_insertion_completion(v, iIdxCur, aRegIdx[0], @@ -1089,8 +1058,6 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ /* Test all NOT NULL constraints. */ for (uint32_t i = 0; i < def->field_count; i++) { - if ((int) i == pTab->iPKey) - continue; if (aiChng && aiChng[i] < 0) { /* Don't bother checking for NOT NULL on columns that do not change */ continue; @@ -1259,10 +1226,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ * needed for proper UNIQUE * constraint handling. */ - if ((int) fieldno == pTab->iPKey) - reg = regNewData; - else - reg = fieldno + regNewData + 1; + reg = fieldno + regNewData + 1; sqlite3VdbeAddOp2(v, OP_SCopy, reg, regIdx + i); VdbeComment((v, "%s", def->fields[fieldno].name)); @@ -1738,8 +1702,6 @@ xferOptimization(Parse * pParse, /* Parser context */ if (space_trigger_list(pDest->def->id) != NULL) return 0; if (onError == ON_CONFLICT_ACTION_DEFAULT) { - if (pDest->iPKey >= 0) - onError = pDest->keyConf; if (onError == ON_CONFLICT_ACTION_DEFAULT) onError = ON_CONFLICT_ACTION_ABORT; } @@ -1800,9 +1762,6 @@ xferOptimization(Parse * pParse, /* Parser context */ /* Number of columns must be the same in src and dst. */ if (pDest->def->field_count != pSrc->def->field_count) return 0; - /* Both tables must have the same INTEGER PRIMARY KEY. */ - if (pDest->iPKey != pSrc->iPKey) - return 0; for (i = 0; i < (int)pDest->def->field_count; i++) { enum affinity_type dest_affinity = pDest->def->fields[i].affinity; @@ -1893,7 +1852,7 @@ xferOptimization(Parse * pParse, /* Parser context */ regTupleid = sqlite3GetTempReg(pParse); sqlite3OpenTable(pParse, iDest, pDest, OP_OpenWrite); assert(destHasUniqueIdx); - if ((pDest->iPKey < 0 && pDest->pIndex != 0) /* (1) */ + if ((pDest->pIndex != 0) /* (1) */ ||destHasUniqueIdx /* (2) */ || (onError != ON_CONFLICT_ACTION_ABORT && onError != ON_CONFLICT_ACTION_ROLLBACK) /* (3) */ diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 7b1f6ec..a258bfc 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -726,22 +726,19 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ int iKey = pFK->aCol[0].iFrom; assert(iKey >= 0 && iKey < (int)pTab->def->field_count); - if (iKey != pTab->iPKey) { - sqlite3VdbeAddOp3(v, - OP_Column, - 0, - iKey, - regRow); - sqlite3ColumnDefault(v, - pTab->def, - iKey, - regRow); - sqlite3VdbeAddOp2(v, - OP_IsNull, - regRow, - addrOk); - VdbeCoverage(v); - } + sqlite3VdbeAddOp3(v, + OP_Column, + 0, + iKey, + regRow); + sqlite3ColumnDefault(v, + pTab->def, + iKey, + regRow); + sqlite3VdbeAddOp2(v, + OP_IsNull, + regRow, + addrOk); VdbeCoverage(v); sqlite3VdbeGoto(v, addrOk); sqlite3VdbeJumpHere(v, diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index a185473..43b2198 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -336,9 +336,6 @@ lookupName(Parse * pParse, /* The parsing context */ iCol++, pCol++) { if (strcmp(pTab->def->fields[iCol].name, zCol) == 0) { - if (iCol == pTab->iPKey) { - iCol = -1; - } break; } } @@ -498,15 +495,11 @@ sqlite3CreateColumnExpr(sqlite3 * db, SrcList * pSrc, int iSrc, int iCol) struct SrcList_item *pItem = &pSrc->a[iSrc]; p->space_def = pItem->pTab->def; p->iTable = pItem->iCursor; - if (pItem->pTab->iPKey == iCol) { - p->iColumn = -1; - } else { - p->iColumn = (ynVar) iCol; - testcase(iCol == BMS); - testcase(iCol == BMS - 1); - pItem->colUsed |= - ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol); - } + p->iColumn = (ynVar) iCol; + testcase(iCol == BMS); + testcase(iCol == BMS - 1); + pItem->colUsed |= + ((Bitmask) 1) << (iCol >= BMS ? BMS - 1 : iCol); ExprSetProperty(p, EP_Resolved); } return p; diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 34d5329..e548021 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1762,8 +1762,6 @@ generateColumnNames(Parse * pParse, /* Parser context */ } assert(j < pTabList->nSrc); pTab = pTabList->a[j].pTab; - if (iCol < 0) - iCol = pTab->iPKey; assert(iCol >= 0 && iCol < (int)pTab->def->field_count); zCol = pTab->def->fields[iCol].name; if (!shortNames && !fullNames) { @@ -2016,7 +2014,6 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * pSelect) assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); sqlite3ColumnsFromExprList(pParse, pSelect->pEList, table); sqlite3SelectAddColumnTypeAndCollation(pParse, table, pSelect); - table->iPKey = -1; if (db->mallocFailed) { sqlite3DeleteTable(db, table); return 0; @@ -4618,7 +4615,6 @@ withExpand(Walker * pWalker, struct SrcList_item *pFrom) if (pTab == NULL) return WRC_Abort; pTab->nTabRef = 1; - pTab->iPKey = -1; pTab->tuple_log_count = DEFAULT_TUPLE_LOG_COUNT; assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); @@ -4821,7 +4817,6 @@ selectExpander(Walker * pWalker, Select * p) sqlite3ColumnsFromExprList(pParse, pSel->pEList, pTab); if (sql_table_def_rebuild(db, pTab) != 0) return WRC_Abort; - pTab->iPKey = -1; pTab->tuple_log_count = DEFAULT_TUPLE_LOG_COUNT; assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index a205b72..9ea7c78 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1918,7 +1918,6 @@ struct Table { Hash idxHash; /* All (named) indices indexed by name */ int tnum; /* Root BTree page for this table */ u32 nTabRef; /* Number of pointers to this Table */ - i16 iPKey; /* If not negative, use aCol[iPKey] as the rowid */ i16 iAutoIncPKey; /* If PK is marked INTEGER PRIMARY KEY AUTOINCREMENT, store column number here, -1 otherwise Tarantool specifics */ /** @@ -1929,7 +1928,6 @@ struct Table { */ LogEst tuple_log_count; u8 tabFlags; /* Mask of TF_* values */ - u8 keyConf; /* What to do in case of uniqueness conflict on iPKey */ #ifndef SQLITE_OMIT_ALTERTABLE int addColOffset; /* Offset in CREATE TABLE stmt to add a new column */ #endif @@ -2096,7 +2094,7 @@ enum sql_index_type { * of this structure may be created. In this case the Index.tnum variable is * used to store the address of a VDBE instruction, not a database page * number (it cannot - the database page is not allocated until the VDBE - * program is executed). See convertToWithoutRowidTable() for details. + * program is executed). */ struct Index { /** The SQL table being indexed. */ @@ -2935,7 +2933,6 @@ struct Parse { Token sLastToken; /* The last token parsed */ ynVar nVar; /* Number of '?' variables seen in the SQL so far */ - u8 iPkSortOrder; /* ASC or DESC for INTEGER PRIMARY KEY */ u8 explain; /* True if the EXPLAIN flag is found on the query */ int nHeight; /* Expression tree height of current sub-select */ int iSelectId; /* ID of current select for EXPLAIN output */ diff --git a/src/box/sql/update.c b/src/box/sql/update.c index 167f425..d51a05c 100644 --- a/src/box/sql/update.c +++ b/src/box/sql/update.c @@ -468,28 +468,24 @@ sqlite3Update(Parse * pParse, /* The parser context */ newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1, TRIGGER_BEFORE, pTab, on_error); for (i = 0; i < (int)def->field_count; i++) { - if (i == pTab->iPKey) { - sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); + j = aXRef[i]; + if (j >= 0) { + sqlite3ExprCode(pParse, pChanges->a[j].pExpr, + regNew + i); + } else if (0 == (tmask & TRIGGER_BEFORE) || i > 31 + || (newmask & MASKBIT32(i))) { + /* This branch loads the value of a column that will not be changed + * into a register. This is done if there are no BEFORE triggers, or + * if there are one or more BEFORE triggers that use this value via + * a new.* reference in a trigger program. + */ + testcase(i == 31); + testcase(i == 32); + sqlite3ExprCodeGetColumnToReg(pParse, def, i, + iDataCur, + regNew + i); } else { - j = aXRef[i]; - if (j >= 0) { - sqlite3ExprCode(pParse, pChanges->a[j].pExpr, - regNew + i); - } else if (0 == (tmask & TRIGGER_BEFORE) || i > 31 - || (newmask & MASKBIT32(i))) { - /* This branch loads the value of a column that will not be changed - * into a register. This is done if there are no BEFORE triggers, or - * if there are one or more BEFORE triggers that use this value via - * a new.* reference in a trigger program. - */ - testcase(i == 31); - testcase(i == 32); - sqlite3ExprCodeGetColumnToReg(pParse, def, i, - iDataCur, - regNew + i); - } else { - sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); - } + sqlite3VdbeAddOp2(v, OP_Null, 0, regNew + i); } } @@ -524,7 +520,7 @@ sqlite3Update(Parse * pParse, /* The parser context */ * registers in case this has happened. */ for (i = 0; i < (int)def->field_count; i++) { - if (aXRef[i] < 0 && i != pTab->iPKey) { + if (aXRef[i] < 0) { sqlite3ExprCodeGetColumnOfTable(v, def, iDataCur, i, regNew + i); diff --git a/src/box/sql/where.c b/src/box/sql/where.c index a4ba629..c115c4a 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -3478,8 +3478,6 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo, /* The WHERE clause */ pIndex->def->key_def; iColumn = def->parts[j].fieldno; revIdx = def->parts[j].sort_order; - if (iColumn == pIndex->pTable->iPKey) - iColumn = -1; } else { iColumn = -1; revIdx = 0; diff --git a/test/sql-tap/analyze6.test.lua b/test/sql-tap/analyze6.test.lua index cb1710a..4bbb67c 100755 --- a/test/sql-tap/analyze6.test.lua +++ b/test/sql-tap/analyze6.test.lua @@ -116,7 +116,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.2> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.2> }) @@ -148,7 +148,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.5> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.5> }) @@ -183,7 +183,7 @@ test:do_eqp_test( [[SELECT * FROM t201 WHERE y=5]], { -- <analyze6-2.8> - {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX sql_autoindex_T201_1 (Y=?)"} + {0, 0, 0, "SEARCH TABLE T201 USING COVERING INDEX unique_unnamed_T201_2 (Y=?)"} -- </analyze6-2.8> }) diff --git a/test/sql-tap/gh-2931-savepoints.test.lua b/test/sql-tap/gh-2931-savepoints.test.lua index 3861bb2..11b4ac5 100755 --- a/test/sql-tap/gh-2931-savepoints.test.lua +++ b/test/sql-tap/gh-2931-savepoints.test.lua @@ -80,7 +80,7 @@ local testcases = { {0,{1,2,10,11,1,2,4,10,11}}}, {"14", [[insert into t1 values(4);]], - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}}, + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}}, {"15", [[select * from t1 union all select * from t2;]], {0,{1,2,10,11,1,2,4,10,11}}}, diff --git a/test/sql-tap/gh2140-trans.test.lua b/test/sql-tap/gh2140-trans.test.lua index fe7af5f..005163a 100755 --- a/test/sql-tap/gh2140-trans.test.lua +++ b/test/sql-tap/gh2140-trans.test.lua @@ -32,7 +32,7 @@ for _, verb in ipairs({'ROLLBACK', 'ABORT'}) do if verb == 'ROLLBACK' then answer = 'UNIQUE constraint failed: T1.S1' else - answer = "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + answer = "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" end test:do_catchsql_test('insert1_'..verb, [[BEGIN; diff --git a/test/sql-tap/gh2259-in-stmt-trans.test.lua b/test/sql-tap/gh2259-in-stmt-trans.test.lua index e2ae169..d1ced19 100755 --- a/test/sql-tap/gh2259-in-stmt-trans.test.lua +++ b/test/sql-tap/gh2259-in-stmt-trans.test.lua @@ -18,7 +18,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'_insert1', 'INSERT INTO t1 VALUES(1, 2)', - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test(prefix..'_insert1_check1', 'SELECT * FROM t1', @@ -34,7 +34,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'_update1', 'UPDATE t1 SET s1=1', - {1,"Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1,"Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test(prefix..'_update1_check1', 'SELECT * FROM t1', @@ -52,7 +52,7 @@ for _, prefix in pairs({"BEFORE", "AFTER"}) do test:do_catchsql_test(prefix..'delete1', 'DELETE FROM t1;', - {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) -- Nothing should be inserted due to abort test:do_execsql_test('delete1_check1', @@ -69,7 +69,7 @@ end -- Check multi-insert test:do_catchsql_test('insert2', 'INSERT INTO t1 VALUES (5, 6), (6, 7)', - {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"}) + {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"}) test:do_execsql_test('insert2_check', 'SELECT * FROM t1;', {3, 3}) diff --git a/test/sql-tap/gh2964-abort.test.lua b/test/sql-tap/gh2964-abort.test.lua index a06b4fd..52809d8 100755 --- a/test/sql-tap/gh2964-abort.test.lua +++ b/test/sql-tap/gh2964-abort.test.lua @@ -13,7 +13,7 @@ test:do_catchsql_test( "CREATE TABLE t2 (a int primary key);") local insert_err = {1, "UNIQUE constraint failed: T2.A"} -local insert_err_PK = {1, "Duplicate key exists in unique index 'sql_autoindex_T2_1' in space 'T2'"} +local insert_err_PK = {1, "Duplicate key exists in unique index 'pk_unnamed_T2_1' in space 'T2'"} local data = { --id|TRIG TYPE|INSERT TYPE|insert error|commit error| result {1, "AFTER", "or abort", insert_err_PK, {0}, {1,1,2}}, diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua index 9ee40fa..78205f8 100755 --- a/test/sql-tap/index1.test.lua +++ b/test/sql-tap/index1.test.lua @@ -444,10 +444,10 @@ test:do_execsql_test( test:do_execsql_test( "index-7.3", [[ - SELECT "name" FROM "_index" WHERE "name"='sql_autoindex_TEST1_1' + SELECT "name" FROM "_index" WHERE "name"='pk_unnamed_TEST1_1' ]], { -- <index-7.3> - "sql_autoindex_TEST1_1" + "pk_unnamed_TEST1_1" -- </index-7.3> }) @@ -1017,7 +1017,7 @@ test:do_execsql_test( SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE "_index"."id" = "_space"."id" AND "_space"."name"='T7'; ]], { -- <index-17.1> - "sql_autoindex_T7_3", "sql_autoindex_T7_2", "sql_autoindex_T7_1" + "pk_unnamed_T7_3", "unique_unnamed_T7_2", "unique_unnamed_T7_1" -- </index-17.1> }) @@ -1071,7 +1071,7 @@ test:do_execsql_test( INSERT INTO t7 VALUES(1); ]], { -- <index-19.2> - 1, "Duplicate key exists in unique index 'sql_autoindex_T7_1' in space 'T7'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T7_1' in space 'T7'" -- </index-19.2> }) diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua index 3914e3c..4f90708 100755 --- a/test/sql-tap/index7.test.lua +++ b/test/sql-tap/index7.test.lua @@ -355,7 +355,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST5'; ]], - {0, {"sql_autoindex_TEST5_1",0}}) + {0, {"pk_unnamed_TEST5_1",0}}) -- This test checks that CREATE TABLE statement with PK constraint -- and NAMED UNIQUE constraint (declared on THE SAME COLUMNS) @@ -371,7 +371,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST6'; ]], - {0, {"sql_autoindex_TEST6_1",0,"unique_constraint_C1",1}}) + {0, {"pk_unnamed_TEST6_1",0,"unique_C1_2",1}}) -- This test checks that CREATE TABLE statement with PK constraint -- and UNIQUE constraint is executed correctly @@ -386,7 +386,7 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST7'; ]], - {0, {"sql_autoindex_TEST7_1",0}}) + {0, {"unique_unnamed_TEST7_1",0}}) -- This test is the same as previous, but with named UNIQUE @@ -401,6 +401,6 @@ test:do_catchsql_test( "_index"."id" = "_space"."id" AND "_space"."name"='TEST8'; ]], - {0, {"sql_autoindex_TEST8_2",0,"unique_constraint_C1",1}}) + {0, {"pk_TEST8_2",0,"unique_C1_1",1}}) test:finish_test() diff --git a/test/sql-tap/intpkey.test.lua b/test/sql-tap/intpkey.test.lua index b5359b6..3193e48 100755 --- a/test/sql-tap/intpkey.test.lua +++ b/test/sql-tap/intpkey.test.lua @@ -42,7 +42,7 @@ test:do_execsql_test( SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE "_index"."id" = "_space"."id" AND "_space"."name"='T1' ]], { -- <intpkey-1.1> - "sql_autoindex_T1_1" + "pk_unnamed_T1_1" -- </intpkey-1.1> }) @@ -96,7 +96,7 @@ test:do_catchsql_test( INSERT INTO t1 VALUES(5,'second','entry'); ]], { -- <intpkey-1.6> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </intpkey-1.6> }) diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index a5078b5..0fae8d1 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -380,7 +380,7 @@ test:do_catchsql_test( INSERT INTO t5 VALUES(1,2,4); ]], { -- <misc1-7.4> - 1, "Duplicate key exists in unique index 'sql_autoindex_T5_1' in space 'T5'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T5_1' in space 'T5'" -- </misc1-7.4> }) diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua index 63b5065..3856d26 100755 --- a/test/sql-tap/unique.test.lua +++ b/test/sql-tap/unique.test.lua @@ -70,7 +70,7 @@ test:do_catchsql_test( INSERT INTO t1(a,b,c) VALUES(1,3,4) ]], { -- <unique-1.3> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_2' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </unique-1.3> }) @@ -91,7 +91,7 @@ test:do_catchsql_test( INSERT INTO t1(a,b,c) VALUES(3,2,4) ]], { -- <unique-1.5> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'" -- </unique-1.5> }) @@ -287,7 +287,7 @@ test:do_catchsql_test( SELECT a,b,c,d FROM t3 ORDER BY a,b,c,d; ]], { -- <unique-3.4> - 1, "Duplicate key exists in unique index 'sql_autoindex_T3_2' in space 'T3'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T3_2' in space 'T3'" -- </unique-3.4> }) @@ -444,7 +444,7 @@ test:do_catchsql_test( INSERT INTO t5 VALUES(2, 1,2,3,4,5,6); ]], { -- <unique-5.2> - 1, "Duplicate key exists in unique index 'sql_autoindex_T5_2' in space 'T5'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T5_2' in space 'T5'" -- </unique-5.2> }) diff --git a/test/sql-tap/update.test.lua b/test/sql-tap/update.test.lua index 1ed951d..d4debc7 100755 --- a/test/sql-tap/update.test.lua +++ b/test/sql-tap/update.test.lua @@ -917,7 +917,7 @@ test:do_catchsql_test("update-10.3", [[ SELECT * FROM t1; ]], { -- <update-10.3> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_3' in space 'T1'" + 1, "Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'" -- </update-10.3> }) @@ -943,7 +943,7 @@ test:do_catchsql_test("update-10.6", [[ SELECT * FROM t1; ]], { -- <update-10.6> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_1' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'" -- </update-10.6> }) @@ -969,7 +969,7 @@ test:do_catchsql_test("update-10.9", [[ SELECT * FROM t1; ]], { -- <update-10.9> - 1, "Duplicate key exists in unique index 'sql_autoindex_T1_2' in space 'T1'" + 1, "Duplicate key exists in unique index 'unique_unnamed_T1_3' in space 'T1'" -- </update-10.9> }) diff --git a/test/sql/insert-unique.result b/test/sql/insert-unique.result index 359ac43..797c8ef 100644 --- a/test/sql/insert-unique.result +++ b/test/sql/insert-unique.result @@ -24,7 +24,7 @@ box.sql.execute("INSERT INTO zoobar VALUES (111, 222, 'c3', 444)") -- PK must be unique box.sql.execute("INSERT INTO zoobar VALUES (112, 222, 'c3', 444)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_ZOOBAR_1' in space 'ZOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_ZOOBAR_1' in space 'ZOOBAR' ... -- Unique index must be respected box.sql.execute("INSERT INTO zoobar VALUES (111, 223, 'c3', 444)") diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 26ad17b..af474bc 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -553,7 +553,7 @@ future1:wait_result() future2:wait_result() --- - null -- 'Failed to execute SQL statement: Duplicate key exists in unique index ''sql_autoindex_TEST_1'' +- 'Failed to execute SQL statement: Duplicate key exists in unique index ''pk_unnamed_TEST_1'' in space ''TEST''' ... future3:wait_result() diff --git a/test/sql/on-conflict.result b/test/sql/on-conflict.result index 4080648..b4772d8 100644 --- a/test/sql/on-conflict.result +++ b/test/sql/on-conflict.result @@ -23,7 +23,7 @@ box.sql.execute("CREATE TABLE e (id INTEGER PRIMARY KEY ON CONFLICT REPLACE, v I -- Insert values and select them box.sql.execute("INSERT INTO t values (1, 1), (2, 2), (3, 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_T_1' in space 'T' +- error: Duplicate key exists in unique index 'unique_unnamed_T_2' in space 'T' ... box.sql.execute("SELECT * FROM t") --- diff --git a/test/sql/persistency.result b/test/sql/persistency.result index f64e666..c65baa0 100644 --- a/test/sql/persistency.result +++ b/test/sql/persistency.result @@ -26,7 +26,7 @@ box.sql.execute("INSERT INTO foobar VALUES (1000, 'foobar')") ... box.sql.execute("INSERT INTO foobar VALUES (1, 'duplicate')") --- -- error: Duplicate key exists in unique index 'sql_autoindex_FOOBAR_1' in space 'FOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_FOOBAR_1' in space 'FOOBAR' ... -- simple select box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar") @@ -208,7 +208,7 @@ box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"") -- prove barfoo2 still exists box.sql.execute("INSERT INTO barfoo VALUES ('xfoo', 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_BARFOO_1' in space 'BARFOO' +- error: Duplicate key exists in unique index 'pk_unnamed_BARFOO_1' in space 'BARFOO' ... box.sql.execute("SELECT * FROM barfoo") --- diff --git a/test/sql/transition.result b/test/sql/transition.result index 765b0f0..805e8aa 100644 --- a/test/sql/transition.result +++ b/test/sql/transition.result @@ -23,7 +23,7 @@ box.sql.execute("INSERT INTO foobar VALUES (1000, 'foobar')") ... box.sql.execute("INSERT INTO foobar VALUES (1, 'duplicate')") --- -- error: Duplicate key exists in unique index 'sql_autoindex_FOOBAR_1' in space 'FOOBAR' +- error: Duplicate key exists in unique index 'pk_unnamed_FOOBAR_1' in space 'FOOBAR' ... -- simple select box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar") @@ -141,7 +141,7 @@ box.sql.execute("INSERT INTO barfoo VALUES ('foobar', 1000)") -- prove barfoo2 was created box.sql.execute("INSERT INTO barfoo VALUES ('xfoo', 1)") --- -- error: Duplicate key exists in unique index 'sql_autoindex_BARFOO_1' in space 'BARFOO' +- error: Duplicate key exists in unique index 'pk_unnamed_BARFOO_1' in space 'BARFOO' ... box.sql.execute("SELECT foo, bar FROM barfoo") --- ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor primary index creation 2018-07-20 13:08 ` Kirill Yukhin @ 2018-07-20 14:34 ` n.pettik 2018-07-20 14:40 ` Kirill Yukhin 0 siblings, 1 reply; 5+ messages in thread From: n.pettik @ 2018-07-20 14:34 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Yukhin [-- Attachment #1: Type: text/plain, Size: 1380 bytes --] LGTM, except for 1 comment. > @@ -862,10 +861,6 @@ sqlite3AddDefaultValue(Parse * pParse, ExprSpan * pSpan) > * a primary key (and this is the second primary key) then create an > * error. > * > - * Set the Table.iPKey field of the table under construction to be the > - * index of the INTEGER PRIMARY KEY column. > - * Table.iPKey is set to -1 if there is no INTEGER PRIMARY KEY. > - * > * If the key is not an INTEGER PRIMARY KEY, then create a unique > * index for the key. No index is created for INTEGER PRIMARY KEYs. > */ > @@ -923,14 +918,24 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */ > (pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) && > sortOrder != SORT_ORDER_DESC) { > assert(autoInc == 0 || autoInc == 1); > - pTab->iPKey = iCol; > - pTab->keyConf = (u8) onError; > if (autoInc) { > pTab->iAutoIncPKey = iCol; > pTab->tabFlags |= TF_Autoincrement; > } > - if (pList) > - pParse->iPkSortOrder = pList->a[0].sort_order; > + struct sqlite3 *db = pParse->db; > + struct ExprList *list; > + struct Token token; > + sqlite3TokenInit(&token, pTab->def->fields[iCol].name); > + list = sql_expr_list_append(db, NULL, > + sqlite3ExprAlloc(db, TK_ID, > + &token, 0)); > + if (list == NULL) > + return; I guess here should also be goto primary_key_exit; [-- Attachment #2: Type: text/html, Size: 42030 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] sql: refactor primary index creation 2018-07-20 14:34 ` n.pettik @ 2018-07-20 14:40 ` Kirill Yukhin 0 siblings, 0 replies; 5+ messages in thread From: Kirill Yukhin @ 2018-07-20 14:40 UTC (permalink / raw) To: n.pettik; +Cc: tarantool-patches Hello Nikita, On 20 июл 17:34, n.pettik wrote: > LGTM, except for 1 comment. Comment fixed and comitted to 2.0 branch. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-20 15:10 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-20 8:33 [tarantool-patches] [PATCH] sql: refactor primary index creation Kirill Yukhin 2018-07-20 12:11 ` [tarantool-patches] " n.pettik 2018-07-20 13:08 ` Kirill Yukhin 2018-07-20 14:34 ` n.pettik 2018-07-20 14:40 ` Kirill Yukhin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox