Skip to content

Commit

Permalink
sql: allow ON CONFLICT REPLACE only for PK index
Browse files Browse the repository at this point in the history 
Constraints which require VDBE bytecode will be executed before making
insertion into Tarantool. For three major constraints - UNIQUE,
FOREIGN-KEY, CHECK the execution order doesn't matter if error action is
not REPLACE, because one constraint violation doesn't affect others.

But violation of constraint with ON CONFLICT REPLACE leads to a
removal of the whole tuple. When only PRIMARY KEY has REPLACE as
error action, it is not a problem and such behavior is absolutely
similar to Tarantool. But when we are dealing with secondary index
NOT NULL constraint with ON CONFLICT REPLACE, conflict in
secondary index leads to a whole tuple removal, and as a result
when there is also a conflict in primary key index, insertion
will lead to success instead of raising error.

In this patch possibility to create constraint non PRIMARY KEY
constraint with ON CONFLICT REPLACE has been banned, that kind of
behavior leads to a proper error message.

Closes #2963
  • Loading branch information
@TheAviat0r @kyukhin
TheAviat0r authored and kyukhin committed May 15, 2018
1 parent b47d163 commit f74f806
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 259 deletions.
43 changes: 43 additions & 0 deletions src/box/sql/build.c
View file
Expand Up @@ -166,6 +166,41 @@ sqlite3NestedParse(Parse * pParse, const char *zFormat, ...)
pParse->nested--;
}

/**
* This is a function which should be called during execution
* of sqlite3EndTable. It ensures that only PRIMARY KEY
* constraint may have ON CONFLICT REPLACE clause.
*
* @param table Space which should be checked.
* @retval False, if only primary key constraint has
* ON CONFLICT REPLACE clause or if there are no indexes
* with REPLACE as error action. True otherwise.
*/
static bool
check_on_conflict_replace_entries(struct Table *table)
{
/* Check all NOT NULL constraints. */
for (int i = 0; i < table->nCol; i++) {
enum on_conflict_action on_error = table->aCol[i].notNull;
if (on_error == ON_CONFLICT_ACTION_REPLACE &&
table->aCol[i].is_primkey == false) {
return true;
}
}
/* Check all UNIQUE constraints. */
for (struct Index *idx = table->pIndex; idx; idx = idx->pNext) {
if (idx->onError == ON_CONFLICT_ACTION_REPLACE &&
!IsPrimaryKeyIndex(idx)) {
return true;
}
}
/*
* CHECK constraints are not allowed to have REPLACE as
* error action and therefore can be skipped.
*/
return false;
}

/*
* Locate the in-memory structure that describes a particular database
* table given the name of that table. Return NULL if not found.
Expand Down Expand Up @@ -1865,6 +1900,14 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
}

if (check_on_conflict_replace_entries(p)) {
sqlite3ErrorMsg(pParse,
"only PRIMARY KEY constraint can "
"have ON CONFLICT REPLACE clause "
"- %s", p->zName);
return;
}

#ifndef SQLITE_OMIT_CHECK
/* Resolve names in all CHECK constraint expressions.
*/
Expand Down
273 changes: 21 additions & 252 deletions test/sql-tap/conflict3.test.lua
View file
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
test:plan(44)
test:plan(29)

--!./tcltestrunner.lua
-- 2013-11-05
Expand Down Expand Up @@ -353,281 +353,50 @@ test:do_execsql_test(
-- </conflict-6.4>
})

-- Change which column is the PRIMARY KEY
--
test:do_execsql_test(
test:do_catchsql_test(
"conflict-7.1",
[[
DROP TABLE t1;
CREATE TABLE t1(
a UNIQUE ON CONFLICT REPLACE,
b INTEGER PRIMARY KEY ON CONFLICT IGNORE,
c UNIQUE ON CONFLICT FAIL
);
INSERT INTO t1(a,b,c) VALUES(1,2,3), (2,3,4);
SELECT a,b,c FROM t1 ORDER BY a;
CREATE TABLE t3(a PRIMARY KEY ON CONFLICT REPLACE,
b UNIQUE ON CONFLICT REPLACE);
]], {
-- <conflict-7.1>
1, 2, 3, 2, 3, 4
-- </conflict-7.1>
1, "only PRIMARY KEY constraint can have ON CONFLICT REPLACE clause - T3"
})

-- Insert a row that conflicts on column B. The insert should be ignored.
--
test:do_execsql_test(
test:do_catchsql_test(
"conflict-7.2",
[[
INSERT INTO t1(a,b,c) VALUES(3,2,5);
SELECT a,b,c FROM t1 ORDER BY a;
CREATE TABLE t3(a PRIMARY KEY,
b UNIQUE ON CONFLICT REPLACE);
]], {
-- <conflict-7.2>
1, 2, 3, 2, 3, 4
-- </conflict-7.2>
1, "only PRIMARY KEY constraint can have ON CONFLICT REPLACE clause - T3"
})

-- Insert two rows where the second conflicts on C. The first row show go
-- and and then there should be a constraint error.
--
test:do_catchsql_test(
"conflict-7.3",
[[
INSERT INTO t1(a,b,c) VALUES(4,5,6), (5,6,4);
]], {
-- <conflict-7.3>
1, "UNIQUE constraint failed: T1.C"
-- </conflict-7.3>
})

test:do_execsql_test(
"conflict-7.4",
[[
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-7.4>
1, 2, 3, 2, 3, 4, 4, 5, 6
-- </conflict-7.4>
})

-- Change which column is the PRIMARY KEY
--
test:do_execsql_test(
"conflict-8.1",
[[
DROP TABLE t1;
CREATE TABLE t1(
a UNIQUE ON CONFLICT REPLACE,
b INT PRIMARY KEY ON CONFLICT IGNORE,
c UNIQUE ON CONFLICT FAIL
);
INSERT INTO t1(a,b,c) VALUES(1,2,3), (2,3,4);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-8.1>
1, 2, 3, 2, 3, 4
-- </conflict-8.1>
})

-- Insert a row that conflicts on column B. The insert should be ignored.
--
test:do_execsql_test(
"conflict-8.2",
[[
INSERT INTO t1(a,b,c) VALUES(3,2,5);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-8.2>
1, 2, 3, 2, 3, 4
-- </conflict-8.2>
})

-- Insert two rows where the second conflicts on C. The first row show go
-- and and then there should be a constraint error.
--
test:do_catchsql_test(
"conflict-8.3",
[[
INSERT INTO t1(a,b,c) VALUES(4,5,6), (5,6,4);
]], {
-- <conflict-8.3>
1, "UNIQUE constraint failed: T1.C"
-- </conflict-8.3>
})

test:do_execsql_test(
"conflict-8.4",
[[
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-8.4>
1, 2, 3, 2, 3, 4, 4, 5, 6
-- </conflict-8.4>
})

-- Change which column is the PRIMARY KEY
--
test:do_execsql_test(
"conflict-9.1",
[[
DROP TABLE t1;
CREATE TABLE t1(
a UNIQUE ON CONFLICT REPLACE,
b INT PRIMARY KEY ON CONFLICT IGNORE,
c UNIQUE ON CONFLICT FAIL
);
INSERT INTO t1(a,b,c) VALUES(1,2,3), (2,3,4);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-9.1>
1, 2, 3, 2, 3, 4
-- </conflict-9.1>
})

-- Insert a row that conflicts on column B. The insert should be ignored.
--
test:do_execsql_test(
"conflict-9.2",
[[
INSERT INTO t1(a,b,c) VALUES(3,2,5);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-9.2>
1, 2, 3, 2, 3, 4
-- </conflict-9.2>
})

-- Insert two rows where the second conflicts on C. The first row show go
-- and and then there should be a constraint error.
--
test:do_catchsql_test(
"conflict-9.3",
[[
INSERT INTO t1(a,b,c) VALUES(4,5,6), (5,6,4);
]], {
-- <conflict-9.3>
1, "UNIQUE constraint failed: T1.C"
-- </conflict-9.3>
})

test:do_execsql_test(
"conflict-9.4",
[[
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-9.4>
1, 2, 3, 2, 3, 4, 4, 5, 6
-- </conflict-9.4>
})

-- Change which column is the PRIMARY KEY
--
test:do_execsql_test(
"conflict-10.1",
[[
DROP TABLE t1;
CREATE TABLE t1(
a UNIQUE ON CONFLICT REPLACE,
b UNIQUE ON CONFLICT IGNORE,
c INTEGER PRIMARY KEY ON CONFLICT FAIL
);
INSERT INTO t1(a,b,c) VALUES(1,2,3), (2,3,4);
SELECT a,b,c FROM t1 ORDER BY a;
CREATE TABLE t3(a PRIMARY KEY,
b UNIQUE ON CONFLICT REPLACE,
c UNIQUE ON CONFLICT REPLACE);
]], {
-- <conflict-10.1>
1, 2, 3, 2, 3, 4
-- </conflict-10.1>
1, "only PRIMARY KEY constraint can have ON CONFLICT REPLACE clause - T3"
})

-- Insert a row that conflicts on column B. The insert should be ignored.
--
test:do_execsql_test(
"conflict-10.2",
[[
INSERT INTO t1(a,b,c) VALUES(3,2,5);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-10.2>
1, 2, 3, 2, 3, 4
-- </conflict-10.2>
})

-- Insert two rows where the second conflicts on C. The first row show go
-- and and then there should be a constraint error.
--
test:do_catchsql_test(
"conflict-10.3",
[[
INSERT INTO t1(a,b,c) VALUES(4,5,6), (5,6,4);
]], {
-- <conflict-10.3>
1, "UNIQUE constraint failed: T1.C"
-- </conflict-10.3>
})

test:do_execsql_test(
"conflict-10.4",
[[
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-10.4>
1, 2, 3, 2, 3, 4, 4, 5, 6
-- </conflict-10.4>
})

-- Change which column is the PRIMARY KEY
--
test:do_execsql_test(
"conflict-11.1",
[[
DROP TABLE t1;
CREATE TABLE t1(
a UNIQUE ON CONFLICT REPLACE,
b UNIQUE ON CONFLICT IGNORE,
c PRIMARY KEY ON CONFLICT FAIL
);
INSERT INTO t1(a,b,c) VALUES(1,2,3), (2,3,4);
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-11.1>
1, 2, 3, 2, 3, 4
-- </conflict-11.1>
})

-- Insert a row that conflicts on column B. The insert should be ignored.
--
test:do_execsql_test(
"conflict-11.2",
"conflict-7.4",
[[
INSERT INTO t1(a,b,c) VALUES(3,2,5);
SELECT a,b,c FROM t1 ORDER BY a;
CREATE TABLE t3(a PRIMARY KEY,
b NOT NULL ON CONFLICT REPLACE DEFAULT 1488);
]], {
-- <conflict-11.2>
1, 2, 3, 2, 3, 4
-- </conflict-11.2>
1, "only PRIMARY KEY constraint can have ON CONFLICT REPLACE clause - T3"
})

-- Insert two rows where the second conflicts on C. The first row show go
-- and and then there should be a constraint error.
--
test:do_catchsql_test(
"conflict-11.3",
"conflict-7.5",
[[
INSERT INTO t1(a,b,c) VALUES(4,5,6), (5,6,4);
CREATE TABLE t3(a PRIMARY KEY ON CONFLICT REPLACE,
b NOT NULL ON CONFLICT REPLACE DEFAULT 1488);
]], {
-- <conflict-11.3>
1, "UNIQUE constraint failed: T1.C"
-- </conflict-11.3>
1, "only PRIMARY KEY constraint can have ON CONFLICT REPLACE clause - T3"
})

test:do_execsql_test(
"conflict-11.4",
[[
SELECT a,b,c FROM t1 ORDER BY a;
]], {
-- <conflict-11.4>
1, 2, 3, 2, 3, 4, 4, 5, 6
-- </conflict-11.4>
})



test:finish_test()
2 changes: 1 addition & 1 deletion test/sql-tap/default.test.lua
View file
Expand Up @@ -103,7 +103,7 @@ test:do_execsql_test(
CREATE TABLE t3(
a INTEGER PRIMARY KEY AUTOINCREMENT,
b INT DEFAULT 12345 UNIQUE NOT NULL CHECK( b>=0 AND b<99999 ),
c VARCHAR(123,456) DEFAULT 'hello' NOT NULL ON CONFLICT REPLACE,
c VARCHAR(123,456) DEFAULT 'hello' NOT NULL,
d REAL,
e FLOATING POINT(5,10) DEFAULT 4.36,
f NATIONAL CHARACTER(15), --COLLATE RTRIM,
Expand Down

0 comments on commit f74f806

Please sign in to comment.