[patches] [PATCH V2] sql: change constraints checks order
Bulat Niatshin
niatshin at tarantool.org
Mon Mar 5 21:28:36 MSK 2018
- Modify constraints checks execution order, if user specified ON
CONFLICT REPLACE
- Execute constraint check with ON CONFLICT REPLACE before BEFORE
triggers, if it fails, then remove conflicting entry and insert new,
after that - execute query as usual.
- Ban an opportunity for user to create table with multiple constraints
with ON CONFLICT REPLACE option.
Closes #2963
---
Branch:
https://github.com/tarantool/tarantool/tree/bn/gh-2963-proper-replace
Issue:
https://github.com/tarantool/tarantool/issues/2963
Changes in V2:
- Fixed remarks as per review by Nikita
- Added several comments about changes
- Rebased branch to current 2.0 state, fixed conflicts
src/box/sql/build.c | 53 ++++
src/box/sql/insert.c | 381 ++++++++++++++++---------
src/box/sql/sqliteInt.h | 22 +-
src/box/sql/update.c | 25 +-
test/sql-tap/gh-2963-multiple-replace.test.lua | 69 +++++
5 files changed, 415 insertions(+), 135 deletions(-)
create mode 100755 test/sql-tap/gh-2963-multiple-replace.test.lua
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 9f45c6224..c8f346a4a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1822,6 +1822,51 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
return first_col;
}
+/* This function will be called during execution of sqlite3EndTable.
+ * It will ensure that only one NOT NULL and UNIQUE constraints
+ * with ON CONFLICT REPLACE clause are specified by user
+ * (true will be returned in that case), otherwise
+ * false will be returned and appropriate error message will be showed.
+ */
+static bool
+check_multiple_replace_entries(Table * table)
+{
+ bool on_replace_used = false;
+ Index * idx;
+ int i;
+
+ /* Check all UNIQUE constraints (which are represented by unique
+ * indexes) on the subject of one specified
+ * ON CONFLICT REPLACE clause.
+ */
+ for (idx = table->pIndex; idx; idx = idx->pNext) {
+ if (idx->onError == ON_CONFLICT_ACTION_REPLACE) {
+ if (on_replace_used == true) {
+ return true;
+ }
+ on_replace_used = true;
+ }
+ }
+
+ on_replace_used = false;
+
+ /* Check all NOT NULL constraints. Iterate through columns, because
+ * all info about NOT NULL is stored inside column structure.
+ *
+ */
+ for (i = 0; i < table->nCol; i++) {
+ u8 on_error = table->aCol[i].notNull;
+ if (on_error == ON_CONFLICT_ACTION_REPLACE) {
+ if (on_replace_used == true) {
+ return true;
+ }
+ on_replace_used = true;
+ }
+ }
+
+ return false;
+}
+
/*
* This routine is called to report the final ")" that terminates
* a CREATE TABLE statement.
@@ -1884,6 +1929,14 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
}
+ if (check_multiple_replace_entries(p)) {
+ sqlite3ErrorMsg(pParse,
+ "Table %s can feature only one "
+ "ON CONFLICT REPLACE constraint",
+ p->zName);
+ return;
+ }
+
#ifndef SQLITE_OMIT_CHECK
/* Resolve names in all CHECK constraint expressions.
*/
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b20a47970..f663158da 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -341,6 +341,9 @@ sqlite3Insert(Parse * pParse, /* Parser context */
int regData; /* register holding first column to insert */
int *aRegIdx = 0; /* One register allocated to each index */
+ u8 isReplace; /* Set to true if constraints may cause a replace */
+ u8 isUseSeek; /* True to use OPFLAG_SEEKRESULT */
+
#ifndef SQLITE_OMIT_TRIGGER
int isView; /* True if attempting to insert into a view */
Trigger *pTrigger; /* List of triggers on pTab, if required */
@@ -663,67 +666,10 @@ sqlite3Insert(Parse * pParse, /* Parser context */
VdbeCoverage(v);
}
- /* Run the BEFORE and INSTEAD OF triggers, if there are any
- */
- endOfLoop = sqlite3VdbeMakeLabel(v);
- if (tmask & TRIGGER_BEFORE) {
- int regCols = sqlite3GetTempRange(pParse, pTab->nCol + 1);
-
- /* Create the new column data
- */
- for (i = j = 0; i < pTab->nCol; i++) {
- if (pColumn) {
- for (j = 0; j < pColumn->nId; j++) {
- if (pColumn->a[j].idx == i)
- break;
- }
- }
- if ((!useTempTable && !pList)
- || (pColumn && j >= pColumn->nId)
- || (pColumn == 0
- && IsOrdinaryHiddenColumn(&pTab->aCol[i]))) {
- if (i == pTab->iAutoIncPKey)
- sqlite3VdbeAddOp2(v, OP_Integer, -1,
- regCols + i + 1);
- else
- sqlite3ExprCode(pParse,
- pTab->aCol[i].pDflt,
- regCols + i + 1);
- } else if (useTempTable) {
- sqlite3VdbeAddOp3(v, OP_Column, srcTab, j,
- regCols + i + 1);
- } else {
- assert(pSelect == 0); /* Otherwise useTempTable is true */
- sqlite3ExprCodeAndCache(pParse,
- pList->a[j].pExpr,
- regCols + i + 1);
- }
- if (pColumn == 0
- && !IsOrdinaryHiddenColumn(&pTab->aCol[i]))
- j++;
- }
-
- /* If this is an INSERT on a view with an INSTEAD OF INSERT trigger,
- * do not attempt any conversions before assembling the record.
- * If this is a real table, attempt conversions as required by the
- * table column affinities.
- */
- if (!isView) {
- sqlite3TableAffinity(v, pTab, regCols + 1);
- }
-
- /* Fire BEFORE or INSTEAD OF triggers */
- sqlite3CodeRowTrigger(pParse, pTrigger, TK_INSERT, 0,
- TRIGGER_BEFORE, pTab,
- regCols - pTab->nCol - 1, onError,
- endOfLoop);
-
- sqlite3ReleaseTempRange(pParse, regCols, pTab->nCol + 1);
- }
-
/* Compute the content of the next row to insert into a range of
* registers beginning at regIns.
*/
+ endOfLoop = sqlite3VdbeMakeLabel(v);
if (!isView) {
if (ipkColumn >= 0) {
if (useTempTable) {
@@ -838,39 +784,105 @@ sqlite3Insert(Parse * pParse, /* Parser context */
iRegStore);
}
}
+ sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur,
+ iIdxCur, regIns, 0,
+ ipkColumn >= 0, onError,
+ endOfLoop, &isReplace, 0,
+ CONSTRAINT_REPLACE_MODE);
+ }
+
+ /* Run the BEFORE and INSTEAD OF triggers, if there are any. */
+ if (tmask & TRIGGER_BEFORE) {
+ int regCols = sqlite3GetTempRange(pParse, pTab->nCol + 1);
+
+ /* Create the new column data
+ */
+ for (i = j = 0; i < pTab->nCol; i++) {
+ if (pColumn) {
+ for (j = 0; j < pColumn->nId; j++) {
+ if (pColumn->a[j].idx == i)
+ break;
+ }
+ }
+ if ((!useTempTable && !pList)
+ || (pColumn && j >= pColumn->nId)
+ || (pColumn == 0
+ && IsOrdinaryHiddenColumn(&pTab->aCol[i]))) {
+ if (i == pTab->iAutoIncPKey)
+ sqlite3VdbeAddOp2(v, OP_Integer, -1,
+ regCols + i + 1);
+ else
+ sqlite3ExprCode(pParse,
+ pTab->aCol[i].pDflt,
+ regCols + i + 1);
+ } else if (useTempTable) {
+ sqlite3VdbeAddOp3(v, OP_Column, srcTab, j,
+ regCols + i + 1);
+ } else {
+ /* Otherwise useTempTable is true */
+ assert(pSelect == 0);
+ sqlite3ExprCodeAndCache(pParse,
+ pList->a[j].pExpr,
+ regCols + i + 1);
+ }
+ if (pColumn == 0
+ && !IsOrdinaryHiddenColumn(&pTab->aCol[i]))
+ j++;
+ }
+
+ /* If this is an INSERT on a view with an INSTEAD OF INSERT
+ * trigger, do not attempt any conversions before
+ * assembling the record.
+ * If this is a real table, attempt conversions
+ * as required by the table column affinities.
+ */
+ if (!isView) {
+ sqlite3TableAffinity(v, pTab, regCols + 1);
+ }
+
+ /* Fire BEFORE or INSTEAD OF triggers */
+ sqlite3CodeRowTrigger(pParse, pTrigger, TK_INSERT, 0,
+ TRIGGER_BEFORE, pTab,
+ regCols - pTab->nCol - 1, onError,
+ endOfLoop);
+
+ sqlite3ReleaseTempRange(pParse, regCols, pTab->nCol + 1);
+ }
+
+ /* Update the count of rows that are inserted. */
+ if ((user_session->sql_flags & SQLITE_CountRows) != 0) {
+ sqlite3VdbeAddOp2(v, OP_AddImm, regRowCount, 1);
+ }
+
+ if (!isView) {
+ /* Set the OPFLAG_USESEEKRESULT flag if either (a) there are
+ * no REPLACE constraints or (b) there are no triggers
+ * and this table is not a
+ * parent table in a foreign key constraint. It is safe
+ * to set the flag in the second case as if any
+ * REPLACE constraint is hit, an OP_Delete or OP_IdxDelete
+ * instruction will be executed on each cursor that is
+ * disturbed. And these instructions both clear the
+ * VdbeCursor.seekResult variable, disabling
+ * the OPFLAG_USESEEKRESULT functionality.
+ */
/* Generate code to check constraints and generate index keys
- and do the insertion.
+ * and do the insertion.
*/
- int isReplace; /* Set to true if constraints may cause a replace */
- int bUseSeek; /* True to use OPFLAG_SEEKRESULT */
sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur,
iIdxCur, regIns, 0,
ipkColumn >= 0, onError,
- endOfLoop, &isReplace, 0);
+ endOfLoop, &isReplace, 0,
+ CONSTRAINT_DEFAULT_MODE);
sqlite3FkCheck(pParse, pTab, 0, regIns, 0);
- /* Set the OPFLAG_USESEEKRESULT flag if either (a) there are no REPLACE
- * constraints or (b) there are no triggers and this table is not a
- * parent table in a foreign key constraint. It is safe to set the
- * flag in the second case as if any REPLACE constraint is hit, an
- * OP_Delete or OP_IdxDelete instruction will be executed on each
- * cursor that is disturbed. And these instructions both clear the
- * VdbeCursor.seekResult variable, disabling the OPFLAG_USESEEKRESULT
- * functionality.
- */
- bUseSeek = isReplace == 0 || (pTrigger == 0 &&
+ isUseSeek = isReplace == 0 || (pTrigger == 0 &&
((user_session->sql_flags &
SQLITE_ForeignKeys) == 0 ||
sqlite3FkReferences(pTab) == 0));
sqlite3CompleteInsertion(pParse, pTab, iIdxCur, aRegIdx,
- bUseSeek, onError);
- }
-
- /* Update the count of rows that are inserted
- */
- if ((user_session->sql_flags & SQLITE_CountRows) != 0) {
- sqlite3VdbeAddOp2(v, OP_AddImm, regRowCount, 1);
+ isUseSeek, onError);
}
if (pTrigger) {
@@ -1017,7 +1029,8 @@ checkConstraintUnchanged(Expr * pExpr, int *aiChng)
* of the pTab->pIndex list.
*
* This routine also generates code to check constraints. NOT NULL,
- * CHECK, and UNIQUE constraints are all checked. If a constraint fails,
+ * CHECK, and UNIQUE constraints are all checked if they need bytecode
+ * for successful execution. If a constraint fails,
* then the appropriate action is performed. There are five possible
* actions: ROLLBACK, ABORT, FAIL, REPLACE, and IGNORE.
*
@@ -1026,11 +1039,16 @@ checkConstraintUnchanged(Expr * pExpr, int *aiChng)
* any ROLLBACK The current transaction is rolled back and
* sqlite3_step() returns immediately with a
* return code of SQLITE_CONSTRAINT.
+ * Constraint verification will be done
+ * by VDBE bytecode.
*
* any ABORT Back out changes from the current command
* only (do not do a complete rollback) then
* cause sqlite3_step() to return immediately
* with SQLITE_CONSTRAINT.
+ * Constraint will be verified by Tarantool,
+ * no need for additional bytecode in that
+ * case.
*
* any FAIL Sqlite3_step() returns immediately with a
* return code of SQLITE_CONSTRAINT. The
@@ -1041,13 +1059,19 @@ checkConstraintUnchanged(Expr * pExpr, int *aiChng)
* row is skipped, without throwing an error.
* Processing continues with the next row.
* (There is an immediate jump to ignoreDest.)
+ * Constraint verification will be done
+ * by VDBE bytecode.
*
* NOT NULL REPLACE The NULL value is replace by the default
* value for that column. If the default value
* is NULL, the action is the same as ABORT.
+ * Constraint verification will be done
+ * by VDBE bytecode.
*
* UNIQUE REPLACE The other row that conflicts with the row
* being inserted is removed.
+ * Constraint verification will be done
+ * by VDBE bytecode.
*
* CHECK REPLACE Illegal. The results in an exception.
*
@@ -1055,6 +1079,23 @@ checkConstraintUnchanged(Expr * pExpr, int *aiChng)
* Or if overrideError==ON_CONFLICT_ACTION_DEFAULT, then the pParse->onError parameter
* is used. Or if pParse->onError==ON_CONFLICT_ACTION_DEFAULT then the onError value
* for the constraint is used.
+ *
+ * There are possible modes in which this function is working:
+ * 1) CONSTRAINT_REPLACE_MODE - It will generate
+ * bytecode for UNIQUE and NOT NULL constraints with ON CONFLICT REPLACE
+ * error action, for other constraints this call will be no-op.
+ * If conflict happens during that stage, the appropriate conflicting
+ * entry will be deleted by bytecode.
+ *
+ * sqlite3GenerateConstraintChecks with CONSTRAINT_MAIN_MODE assumes
+ * that it was called before generating bytecode for firing BEFORE triggers.
+ *
+ * 2) CONSTRAINT_DEFAULT_MODE - function with that mode argument will
+ * generate bytecode for all constraints, including the ones with
+ * ON CONFLICT REPLACE error action. For constraints with REPLACE action,
+ * it assumes that possible conflicting entry was deleted before,
+ * but unpresence still will be ensured with ABORT action. For other
+ * error actions type everything is usual.
*/
void
sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
@@ -1065,10 +1106,11 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
int regNewData, /* First register in a range holding values to insert */
int regOldData, /* Previous content. 0 for INSERTs */
u8 pkChng, /* Non-zero if the PRIMARY KEY changed */
- u8 overrideError, /* Override onError to this if not ON_CONFLICT_ACTION_DEFAULT */
- int ignoreDest, /* Jump to this label on an ON_CONFLICT_ACTION_IGNORE resolution */
- int *pbMayReplace, /* OUT: Set to true if constraint may cause a replace */
- int *aiChng) /* column i is unchanged if aiChng[i]<0 */
+ u8 overrideError, /* Override onError to this if not OE_Default */
+ int ignoreDest, /* Jump to this label on an OE_Ignore resolution */
+ u8 *pbMayReplace, /* OUT: Set to true if constraint may cause a replace */
+ int *aiChng, /* column i is unchanged if aiChng[i]<0 */
+ int mode) /* Type of working mode - REPLACE or DEFAULT */
{
Vdbe *v; /* VDBE under constrution */
Index *pIdx; /* Pointer to one of the indices */
@@ -1077,7 +1119,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
int i; /* loop counter */
int ix; /* Index loop counter */
int nCol; /* Number of columns */
- int onError; /* Conflict resolution strategy */
+ int onError; /* Conflict resolution strategy */
int addr1; /* Address of jump instruction */
int seenReplace = 0; /* True if REPLACE is used to resolve INT PK conflict */
int nPkField; /* Number of fields in PRIMARY KEY. */
@@ -1106,7 +1148,8 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
continue;
}
if (aiChng && aiChng[i] < 0) {
- /* Don't bother checking for NOT NULL on columns that do not change */
+ /* Don't bother checking for NOT NULL on columns
+ * that do not change. */
continue;
}
if (table_column_is_nullable(pTab, i)
@@ -1120,15 +1163,52 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
} else if (onError == ON_CONFLICT_ACTION_DEFAULT) {
onError = ON_CONFLICT_ACTION_ABORT;
}
- if (onError == ON_CONFLICT_ACTION_REPLACE
+
+ /* During check in REPLACE mode all constraints
+ * whose error action is not REPLACE should be
+ * skipped.
+ */
+ if (mode == CONSTRAINT_REPLACE_MODE &&
+ onError != ON_CONFLICT_ACTION_REPLACE) {
+ continue;
+ }
+
+ /* When we check all NOT NULL constraints during
+ * UPDATE/INSERT, onError action for constraints
+ * with ON CONFLICT REPLACE should be ABORT,
+ * because REPLACE actions have been already
+ * done in the beginning of insertion/update.
+ */
+ if (mode == CONSTRAINT_DEFAULT_MODE &&
+ onError == ON_CONFLICT_ACTION_REPLACE) {
+ onError = ON_CONFLICT_ACTION_ABORT;
+ }
+
+ /* If DEFAULT value is not assigned for
+ * constraint with REPLACE error action, then
+ * error action should turn into ABORT.
+ */
+ if (mode == CONSTRAINT_REPLACE_MODE &&
+ onError == ON_CONFLICT_ACTION_REPLACE
&& pTab->aCol[i].pDflt == 0) {
onError = ON_CONFLICT_ACTION_ABORT;
}
- assert(onError == ON_CONFLICT_ACTION_ROLLBACK
- || onError == ON_CONFLICT_ACTION_ABORT
- || onError == ON_CONFLICT_ACTION_FAIL
- || onError == ON_CONFLICT_ACTION_IGNORE
- || onError == ON_CONFLICT_ACTION_REPLACE);
+
+ /* Assert that one of conditions below is true:
+ * 1) if mode is CONSTRAINT_DEFAULT_MODE, then check
+ * that error action is ROLLBACK, ABORT, FAIL or IGNORE.
+ * 2) if error action is CONSTRAINT_REPLACE_MODE,
+ * then check that error action is REPLACE or ABORT.
+ */
+ assert((mode == CONSTRAINT_DEFAULT_MODE && /* 1) */
+ (onError == ON_CONFLICT_ACTION_ROLLBACK ||
+ onError == ON_CONFLICT_ACTION_ABORT ||
+ onError == ON_CONFLICT_ACTION_FAIL ||
+ onError == ON_CONFLICT_ACTION_IGNORE)) ||
+ (mode == CONSTRAINT_REPLACE_MODE && /* 2) */
+ (onError == ON_CONFLICT_ACTION_ABORT ||
+ onError == ON_CONFLICT_ACTION_REPLACE)));
+
switch (onError) {
case ON_CONFLICT_ACTION_ABORT:
sqlite3MayAbort(pParse);
@@ -1167,11 +1247,10 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
}
}
- /* Test all CHECK constraints
- */
+ /* Test all CHECK constraint (will be done only in DEFAULT mode). */
#ifndef SQLITE_OMIT_CHECK
- if (pTab->pCheck && (user_session->sql_flags &
- SQLITE_IgnoreChecks) == 0) {
+ if (mode == CONSTRAINT_DEFAULT_MODE && pTab->pCheck &&
+ (user_session->sql_flags & SQLITE_IgnoreChecks) == 0) {
ExprList *pCheck = pTab->pCheck;
pParse->ckBase = regNewData + 1;
onError =
@@ -1203,7 +1282,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
sqlite3VdbeResolveLabel(v, allOk);
}
}
-#endif /* !defined(SQLITE_OMIT_CHECK) */
+#endif /* !defined(SQLITE_OMIT_CHECK) */
/* Test all UNIQUE constraints by creating entries for each UNIQUE
* index and making sure that duplicate entries do not already exist.
@@ -1227,12 +1306,49 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
if (aRegIdx[ix] == 0)
continue; /* Skip indices that do not change */
+
+ iThisCur = iIdxCur + ix;
+ addrUniqueOk = sqlite3VdbeMakeLabel(v);
+
+ /* Find out what action to take in case
+ * there is a uniqueness conflict.
+ */
+ onError = pIdx->onError;
+ if (!IsUniqueIndex(pIdx)) {
+ sqlite3VdbeResolveLabel(v, addrUniqueOk);
+ continue; /* pIdx is not a UNIQUE index */
+ }
+
+ if (overrideError != ON_CONFLICT_ACTION_DEFAULT) {
+ onError = overrideError;
+ } else if (onError == ON_CONFLICT_ACTION_DEFAULT) {
+ onError = ON_CONFLICT_ACTION_ABORT;
+ }
+
+ /* When we check all UNIQUE constraints during UPDATE/INSERT,
+ * onError action for constraints with ON CONFLICT REPLACE
+ * should be ABORT, because REPLACE actions have been already
+ * done in the beginning of insertion/update.
+ */
+ if (mode == CONSTRAINT_DEFAULT_MODE &&
+ onError == ON_CONFLICT_ACTION_REPLACE) {
+ onError = ON_CONFLICT_ACTION_ABORT;
+ }
+
+ /* If we are checking ON CONFLICT REPLACE before UPDATE/INSERT,
+ * then skip iteration if onError is not OE_Replace
+ */
+ if (mode == CONSTRAINT_REPLACE_MODE &&
+ onError != ON_CONFLICT_ACTION_REPLACE) {
+ sqlite3VdbeResolveLabel(v, addrUniqueOk);
+ continue;
+ }
+
+
if (bAffinityDone == 0) {
sqlite3TableAffinity(v, pTab, regNewData+1);
bAffinityDone = 1;
}
- iThisCur = iIdxCur + ix;
- addrUniqueOk = sqlite3VdbeMakeLabel(v);
/* Skip partial indices for which the WHERE clause is not true */
if (pIdx->pPartIdxWhere) {
@@ -1244,7 +1360,8 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
}
/* Create a record for this index entry as it should appear after
- * the insert or update. Store that record in the aRegIdx[ix] register
+ * the insert or update. Store that record in the
+ * aRegIdx[ix] register.
*/
regIdx = aRegIdx[ix] + 1;
for (i = 0; i < pIdx->nColumn; i++) {
@@ -1315,15 +1432,12 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
continue;
}
- /* Find out what action to take in case there is a uniqueness conflict */
- onError = pIdx->onError;
- if (onError == ON_CONFLICT_ACTION_NONE) {
- sqlite3VdbeResolveLabel(v, addrUniqueOk);
- continue; /* pIdx is not a UNIQUE index */
- }
- /* If pIdx is not a UNIQUE or we are doing INSERT OR IGNORE,
- * INSERT OR FAIL then skip uniqueness checks and let it to be
+ /* If pIdx we are doing INSERT/UPDATE OR IGNORE,
+ * INSERT/UPDATAE OR FAIL then skip uniqueness checks and let it to be
* done by Tarantool.
+ *
+ * It is placed after adding OP_MakeRecord opcode, because
+ * it will be necessary for OP_IdxInsert.
*/
if (overrideError == ON_CONFLICT_ACTION_FAIL ||
overrideError == ON_CONFLICT_ACTION_IGNORE ||
@@ -1331,23 +1445,23 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
sqlite3VdbeResolveLabel(v, addrUniqueOk);
continue; /* pIdx is not a UNIQUE index */
}
- if (overrideError != ON_CONFLICT_ACTION_DEFAULT) {
- onError = overrideError;
- } else if (onError == ON_CONFLICT_ACTION_DEFAULT) {
- onError = ON_CONFLICT_ACTION_ABORT;
- }
- /* Collision detection may be omitted if all of the following are true:
- * (1) The conflict resolution algorithm is REPLACE
- * (2) There are no secondary indexes on the table
- * (3) No delete triggers need to be fired if there is a conflict
- * (4) No FK constraint counters need to be updated if a conflict occurs.
+ /* Collision detection may be omitted if all of the
+ * following are true:
+ * (1) The conflict resolution algorithm is REPLACE.
+ * (2) There are no secondary indexes on the table.
+ * (3) No delete triggers need to be fired if there
+ * is a conflict.
+ * (4) No FK constraint counters need to be updated
+ * if a conflict occurs.
*/
- if ((ix == 0 && pIdx->pNext == 0) /* Condition 2 */
- && onError == ON_CONFLICT_ACTION_REPLACE /* Condition 1 */
- && (0 == (user_session->sql_flags & SQLITE_RecTriggers) /* Condition 3 */
+ if ((ix == 0 && pIdx->pNext == 0) && /* Condition 2 */
+ onError == ON_CONFLICT_ACTION_REPLACE /* Condition 1 */
+ /* Condition 3 */
+ && (0 == (user_session->sql_flags & SQLITE_RecTriggers)
||0 == sqlite3TriggersExist(pTab, TK_DELETE, 0, 0))
- && (0 == (user_session->sql_flags & SQLITE_ForeignKeys) || /* Condition 4 */
+ /* Condition 4 */
+ && (0 == (user_session->sql_flags & SQLITE_ForeignKeys) ||
(0 == pTab->pFKey && 0 == sqlite3FkReferences(pTab)))
) {
sqlite3VdbeResolveLabel(v, addrUniqueOk);
@@ -1420,12 +1534,23 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
}
}
- /* Generate code that executes if the new index entry is not unique */
- assert(onError == ON_CONFLICT_ACTION_ROLLBACK
- || onError == ON_CONFLICT_ACTION_ABORT
- || onError == ON_CONFLICT_ACTION_FAIL
- || onError == ON_CONFLICT_ACTION_IGNORE
- || onError == ON_CONFLICT_ACTION_REPLACE);
+ /* Generate code that executes if the new index entry is
+ * not unique.
+ * Assert that one of conditions below is true:
+ * 1) if mode is CONSTRAINT_DEFAULT_MODE, then check
+ * that error action is ROLLBACK, ABORT, FAIL or IGNORE.
+ * 2) if error action is CONSTRAINT_REPLACE_MODE,
+ * then check that error action is REPLACE or ABORT.
+ */
+ assert((mode == CONSTRAINT_DEFAULT_MODE &&
+ (onError == ON_CONFLICT_ACTION_ROLLBACK ||
+ onError == ON_CONFLICT_ACTION_ABORT ||
+ onError == ON_CONFLICT_ACTION_FAIL ||
+ onError == ON_CONFLICT_ACTION_IGNORE)) ||
+ (mode == CONSTRAINT_REPLACE_MODE &&
+ (onError == ON_CONFLICT_ACTION_ABORT ||
+ onError == ON_CONFLICT_ACTION_REPLACE)));
+
switch (onError) {
case ON_CONFLICT_ACTION_FAIL:
case ON_CONFLICT_ACTION_ROLLBACK: {
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index aebb61029..a4e69847b 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1536,6 +1536,26 @@ struct FKey {
#define OE_SetDflt 8 /* Set the foreign key value to its default */
#define OE_Cascade 9 /* Cascade the changes */
+
+/* There are two modes in sqlite3GenerateConstraintChecks function.
+ * (a) CONSTRAINT_REPLACE_MODE:
+ * It will generate bytecode for
+ * constraint with ON CONFLICT REPLACE clause, for other constraints with
+ * other onError option sqlite3GenerateConstraintChecks will be no-op.
+ * sqlite3GenerateConstraintChecks with REPLACE MODE should be called
+ * before running BEFORE triggers.
+ * (b) CONSTRAINT_DEFAULT_MODE:
+ * sqlite3GenerateConstraintChecks with that mode argument will
+ * generate required bytecode for all constraints, including the one
+ * that have ON CONFLICT REPLACE clause. But in that case if error happens,
+ * REPLACE action won't be executed, instead of this error action will be
+ * ABORT, because REPLACE for that constraint should be done earlier,
+ * when sqlite3GenerateConstraintChecks was called in
+ * CONSTRAINT_REPLACE_MODE
+ */
+#define CONSTRAINT_REPLACE_MODE 1
+#define CONSTRAINT_DEFAULT_MODE 2
+
/*
* An instance of the following structure is passed as the first
* argument to sqlite3VdbeKeyCompare and is used to control the
@@ -3262,7 +3282,7 @@ int sqlite3GenerateIndexKey(Parse *, Index *, int, int, int, int *, Index *,
int);
void sqlite3ResolvePartIdxLabel(Parse *, int);
void sqlite3GenerateConstraintChecks(Parse *, Table *, int *, int, int, int,
- int, u8, u8, int, int *, int *);
+ int, u8, u8, int, u8 *, int *, int);
void sqlite3CompleteInsertion(Parse *, Table *, int, int *, int, u8);
int sqlite3OpenTableAndIndices(Parse *, Table *, int, u8, int, u8 *, int *,
int *, u8, u8);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 85d18cbde..cd17a202d 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -546,6 +546,18 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
}
+ if (!isView) {
+ u8 isReplace = 0; /* True if REPLACE conflict resolution might happen */
+
+ /* Do constraint checks. */
+ assert(regOldPk > 0);
+ sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur,
+ iIdxCur, regNewPk,
+ regOldPk, chngPk, onError,
+ labelContinue, &isReplace,
+ aXRef, CONSTRAINT_REPLACE_MODE);
+ }
+
/* Fire any BEFORE UPDATE triggers. This happens before constraints are
* verified. One could argue that this is wrong.
*/
@@ -586,16 +598,17 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
if (!isView) {
- int addr1 = 0; /* Address of jump instruction */
- int bReplace = 0; /* True if REPLACE conflict resolution might happen */
+ int addr1 = 0; /* Address of jump instruction. */
+ u8 isReplace = 0; /* True if REPLACE conflict resolution
+ might happen. */
/* Do constraint checks. */
assert(regOldPk > 0);
sqlite3GenerateConstraintChecks(pParse, pTab, aRegIdx, iDataCur,
iIdxCur, regNewPk,
regOldPk, chngPk, onError,
- labelContinue, &bReplace,
- aXRef);
+ labelContinue, &isReplace,
+ aXRef, CONSTRAINT_DEFAULT_MODE);
/* Do FK constraint checks. */
if (hasFK) {
@@ -603,7 +616,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
}
/* Delete the index entries associated with the current record. */
- if (bReplace || chngPk) {
+ if (isReplace || chngPk) {
addr1 =
sqlite3VdbeAddOp4Int(v, OP_NotFound,
iDataCur, 0, regKey,
@@ -636,7 +649,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
sqlite3VdbeAddOp2(v, OP_Delete, iDataCur, 0);
}
#endif
- if (bReplace || chngPk) {
+ if (isReplace || chngPk) {
sqlite3VdbeJumpHere(v, addr1);
}
diff --git a/test/sql-tap/gh-2963-multiple-replace.test.lua b/test/sql-tap/gh-2963-multiple-replace.test.lua
new file mode 100755
index 000000000..0b907173a
--- /dev/null
+++ b/test/sql-tap/gh-2963-multiple-replace.test.lua
@@ -0,0 +1,69 @@
+#!/usr/bin/env tarantool
+-- In SQLite multiple ON CONFLICT REPLACE were allowed in CREATE TABLE statement.
+-- However, we decided that we should have only one constraint with ON CONFLICT
+-- REPLACE action. This tests check that proper error message will be raised
+-- if user makes multiple columns with ON CONFLICT REPLACE.
+
+test = require("sqltester")
+test:plan(5)
+
+test:do_execsql_test(
+ "replace-1.1",
+ [[
+ CREATE TABLE t1(a INT PRIMARY KEY,
+ b INT UNIQUE ON CONFLICT REPLACE,
+ c INT UNIQUE
+ );
+ ]], {
+
+ })
+
+test:do_catchsql_test(
+ "replace-1.2",
+ [[
+ DROP TABLE IF EXISTS t1;
+ CREATE TABLE t1(a INT PRIMARY KEY,
+ b INT UNIQUE ON CONFLICT REPLACE,
+ c INT UNIQUE ON CONFLICT REPLACE
+ );
+ ]], {
+ 1, "Table T1 can feature only one ON CONFLICT REPLACE constraint"
+ })
+
+test:do_execsql_test(
+ "replace-1.3",
+ [[
+ DROP TABLE IF EXISTS t1;
+ CREATE TABLE t1(a INT PRIMARY KEY,
+ b INT UNIQUE ON CONFLICT REPLACE,
+ c INT NOT NULL ON CONFLICT REPLACE
+ );
+ ]], {
+
+ })
+
+test:do_execsql_test(
+ "replace-1.4",
+ [[
+ DROP TABLE IF EXISTS t1;
+ CREATE TABLE t1(a INT PRIMARY KEY,
+ b INT NOT NULL ON CONFLICT REPLACE,
+ c INT UNIQUE ON CONFLICT REPLACE
+ );
+ ]], {
+
+ })
+
+test:do_catchsql_test(
+ "replace-1.5",
+ [[
+ DROP TABLE IF EXISTS t1;
+ CREATE TABLE t1(a INT PRIMARY KEY,
+ b INT NOT NULL ON CONFLICT REPLACE,
+ c INT NOT NULL ON CONFLICT REPLACE
+ );
+ ]], {
+ 1, "Table T1 can feature only one ON CONFLICT REPLACE constraint"
+ })
+
+test:finish_test()
--
2.14.1
More information about the Tarantool-patches
mailing list