* [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