Skip to content

Commit

Permalink
sql: ban multiple ON CONFLICT REPLACE
Browse files Browse the repository at this point in the history
Ban an opportunity for user to create table with multiple constraints
with ON CONFLICT REPLACE option.

For #2963
  • Loading branch information
TheAviat0r committed Feb 12, 2018
1 parent 583865b commit 11bed56
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 0 deletions.
36 changes: 36 additions & 0 deletions src/box/sql/build.c
Expand Up @@ -1822,6 +1822,35 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
return first_col;
}

static bool
checkMultipleReplaceEntries(Table * pTab)
{
bool onReplaceUsed = false;
Index * pIdx;
int i;

for (pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext) {
if (pIdx->onError == OE_Replace) {
if (onReplaceUsed == true) {
return true;
}
onReplaceUsed = true;
}
}

for (i = 0; i < pTab->nCol; i++) {
u8 onError = pTab->aCol[i].notNull;
if (onError == OE_Replace) {
if (onReplaceUsed == true) {
return true;
}
onReplaceUsed = true;
}
}

return false;
}

/*
* This routine is called to report the final ")" that terminates
* a CREATE TABLE statement.
Expand Down Expand Up @@ -1884,6 +1913,13 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
}

if (checkMultipleReplaceEntries(p)) {
sqlite3ErrorMsg(pParse,
"in table %s - only one ON CONFLICT REPLACE is allowed",
p->zName);
return;
}

#ifndef SQLITE_OMIT_CHECK
/* Resolve names in all CHECK constraint expressions.
*/
Expand Down
70 changes: 70 additions & 0 deletions test/sql-tap/gh-2963-multiple-replace.test.lua
@@ -0,0 +1,70 @@
#!/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, "in table T1 - only one ON CONFLICT REPLACE is allowed"
})

test:do_catchsql_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
);
]], {
1, "in table T1 - only one ON CONFLICT REPLACE is allowed"
})

test:do_catchsql_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
);
]], {
1, "in table T1 - only one ON CONFLICT REPLACE is allowed"
})

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, "in table T1 - only one ON CONFLICT REPLACE is allowed"
})

test:finish_test()

21 changes: 21 additions & 0 deletions test/sql/gh2963-multiple-replace.result
@@ -0,0 +1,21 @@
-- gh-2963 - this test is a part of that ticket
-- The purpose is to ban an opportunity for user to create table with multiple
-- ON CONFLICT clauses for columns, user should be able to point it only to one
-- column. This test checks it.
test_run = require('test_run').new()
---
...
box.sql.execute('CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE ON CONFLICT REPLACE, c UNIQUE)')
---
...
box.sql.execute('CREATE TABLE t2(a INT PRIMARY KEY, b INT UNIQUE ON CONFLICT REPLACE, c UNIQUE ON CONFLICT REPLACE)')
---
- error: in table T2 - only one ON CONFLICT REPLACE is allowed
...
box.sql.execute('DROP TABLE t1')
---
...
box.sql.execute('DROP TABLE t2')
---
- error: 'no such table: T2'
...
13 changes: 13 additions & 0 deletions test/sql/gh2963-multiple-replace.test.lua
@@ -0,0 +1,13 @@
-- gh-2963 - this test is a part of that ticket
-- The purpose is to ban an opportunity for user to create table with multiple
-- ON CONFLICT clauses for columns, user should be able to point it only to one
-- column. This test checks it.

test_run = require('test_run').new()

box.sql.execute('CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE ON CONFLICT REPLACE, c UNIQUE)')

box.sql.execute('CREATE TABLE t2(a INT PRIMARY KEY, b INT UNIQUE ON CONFLICT REPLACE, c UNIQUE ON CONFLICT REPLACE)')

box.sql.execute('DROP TABLE t1')
box.sql.execute('DROP TABLE t2')

0 comments on commit 11bed56

Please sign in to comment.