Skip to content

Keep constraints when editing tables that use ON CONFLICT on CHECK or NULL #4242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deepsidhu1313 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Keep constraints when editing tables that use ON CONFLICT on CHECK or NULL #4242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2dbaa88
Fix test-sqlobjects compilation with Qt6
claude Oct 30, 2026
08d8baa
Add tests for ON CONFLICT clauses in table definitions
claude Oct 30, 2026
cc37ea7
Parse ON CONFLICT clauses on table CHECK and NULL constraints
claude Oct 30, 2026
88a02ec
Keep the conflict action when changing the type of an index constraint
claude Oct 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/EditTableDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,20 @@ void EditTableDialog::populateIndexConstraints()
{
auto pk = std::make_shared<sqlb::PrimaryKeyConstraint>();
pk->setName(it.second->name());
pk->setConflictAction(it.second->conflictAction());
m_table.addConstraint(it.first, pk);
} else if(type->itemData(index).toString() == "u") {
auto u = std::make_shared<sqlb::UniqueConstraint>();
u->setName(it.second->name());
u->setConflictAction(it.second->conflictAction());
m_table.addConstraint(it.first, u);
}

// The conflict action is carried over to the new constraint, so make sure the primary key's combo box shows the current one
ui->comboOnConflict->blockSignals(true);
ui->comboOnConflict->setCurrentText(m_table.primaryKey() ? QString::fromStdString(m_table.primaryKey()->conflictAction()).toUpper() : QString());
ui->comboOnConflict->blockSignals(false);

// Update SQL and view
populateFields();
populateIndexConstraints();
Expand Down
1,223 changes: 608 additions & 615 deletions src/sql/parser/sqlite3_parser.cpp
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/sql/parser/sqlite3_parser.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3865,7 +3865,7 @@ switch (yykind)
/// Constants.
enum
{
yylast_ = 3721, ///< Last index in yytable_.
yylast_ = 3687, ///< Last index in yytable_.
yynnts_ = 57, ///< Number of nonterminal symbols.
yyfinal_ = 13 ///< Termination state number.
};
Expand Down
5 changes: 3 additions & 2 deletions src/sql/parser/sqlite3_parser.yy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ columnconstraint:
nn->setConflictAction($4);
drv.last_table_column->setNotNull(nn);
}
| optional_constraintname NULL {
| optional_constraintname NULL optional_conflictclause { // A conflict clause is allowed here but has no effect
}
| optional_constraintname UNIQUE optional_conflictclause {
auto u = std::make_shared<sqlb::UniqueConstraint>();
Expand Down Expand Up @@ -870,9 +870,10 @@ tableconstraint:
sqlb::StringVector columns;
$$.index.insert(std::make_pair($4, u));
}
| optional_constraintname CHECK "(" expr ")" {
| optional_constraintname CHECK "(" expr ")" optional_conflictclause {
auto c = std::make_shared<sqlb::CheckConstraint>($4);
c->setName($1);
c->setConflictAction($6);
$$.check.push_back(c);
}
| optional_constraintname FOREIGN KEY "(" columnid_list ")" REFERENCES tableid optional_columnid_with_paren_list optional_fk_clause {
Expand Down
3 changes: 3 additions & 0 deletions src/sql/sqlitetypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ std::string CheckConstraint::toSql() const
result = "CONSTRAINT " + escapeIdentifier(m_name) + " ";
result += "CHECK(" + m_expression + ")";

if(!m_conflictAction.empty())
result += " ON CONFLICT " + m_conflictAction;

return result;
}

Expand Down
5 changes: 5 additions & 0 deletions src/sql/sqlitetypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,15 @@ class CheckConstraint : public Constraint
void setExpression(const std::string& expr) { m_expression = expr; }
const std::string& expression() const { return m_expression; }

// SQLite accepts a conflict clause for table CHECK constraints but doesn't act on it. We keep it to not change the table definition.
void setConflictAction(const std::string& conflict) { m_conflictAction = conflict; }
const std::string& conflictAction() const { return m_conflictAction; }

std::string toSql() const;

private:
std::string m_expression;
std::string m_conflictAction;
};

class DefaultConstraint : public Constraint
Expand Down
204 changes: 202 additions & 2 deletions src/tests/testsqlobjects.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ void TestTable::parseSQL()
Table tab(*Table::parseSQL(sSQL));

QCOMPARE(tab.name(), "hero");
QCOMPARE(tab.rowidColumns(), {"_rowid_"});
QCOMPARE(tab.rowidColumns(), sqlb::StringVector({"_rowid_"}));
QCOMPARE(tab.fields.at(0).name(), "id");
QCOMPARE(tab.fields.at(1).name(), "name");
QCOMPARE(tab.fields.at(2).name(), "info");
Expand Down Expand Up @@ -386,7 +386,7 @@ void TestTable::parseSQLWithoutRowid()
Table tab(*Table::parseSQL(sSQL));

QCOMPARE(tab.primaryKeyColumns().at(0).name(), "a");
QCOMPARE(tab.rowidColumns(), {"a"});
QCOMPARE(tab.rowidColumns(), sqlb::StringVector({"a"}));
QCOMPARE(tab.withoutRowidTable(), true);
QCOMPARE(tab.isStrict(), false);
}
Expand Down Expand Up @@ -662,6 +662,206 @@ void TestTable::parseIdentifierWithDollar()
QCOMPARE(tab.fields.at(0).type(), "INTEGER");
}

void TestTable::onConflictAccessors()
{
TablePtr tab = Table::parseSQL("CREATE TABLE t(a INTEGER, b TEXT, c TEXT, PRIMARY KEY(a) ON CONFLICT replace, UNIQUE(b) ON CONFLICT Fail, UNIQUE(c))");
QVERIFY(tab->fullyParsed());

// The conflict action of the primary key and of the unique constraints is available through the model, case insensitive
QVERIFY(tab->primaryKey());
QCOMPARE(QString::fromStdString(tab->primaryKey()->conflictAction()).toUpper(), QString("REPLACE"));

const auto constraints = tab->indexConstraints();
QCOMPARE(constraints.size(), size_t(3));
for(const auto& it : constraints)
{
if(it.second->isPrimaryKey())
continue;
if(it.first.front().name() == "b")
QCOMPARE(QString::fromStdString(it.second->conflictAction()).toUpper(), QString("FAIL"));
else
QVERIFY(it.second->conflictAction().empty()); // No ON CONFLICT clause means no conflict action
}
}

void TestTable::onConflictRoundTrip()
{
QFETCH(std::string, sql);
QFETCH(std::string, expected);

// Parsing a table definition with ON CONFLICT clauses must not lose them when writing the table out again
TablePtr parsed = Table::parseSQL(sql);
QVERIFY(parsed->fullyParsed());
QCOMPARE(QString::fromStdString(parsed->sql()), QString::fromStdString(expected));

// The same goes for a copy of the table which is what the Edit Table dialog works on
Table copy(*parsed);
QCOMPARE(QString::fromStdString(copy.sql()), QString::fromStdString(expected));

// And the output must be parseable again and describe the same table
TablePtr reparsed = Table::parseSQL(parsed->sql());
QVERIFY(reparsed->fullyParsed());
QCOMPARE(QString::fromStdString(reparsed->sql()), QString::fromStdString(expected));
}

void TestTable::onConflictRoundTrip_data()
{
QTest::addColumn<std::string>("sql");
QTest::addColumn<std::string>("expected");

// Column constraints. Primary keys are converted to table constraints when parsing.
QTest::newRow("column constraints")
<< std::string("CREATE TABLE t(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b TEXT NOT NULL ON CONFLICT IGNORE, c TEXT UNIQUE ON CONFLICT FAIL)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tTEXT NOT NULL ON CONFLICT IGNORE,\n"
"\t\"c\"\tTEXT UNIQUE ON CONFLICT FAIL,\n"
"\tPRIMARY KEY(\"a\") ON CONFLICT REPLACE\n"
");");

// Table constraints
QTest::newRow("table constraints")
<< std::string("CREATE TABLE t(a INTEGER, b TEXT, c TEXT, PRIMARY KEY(a) ON CONFLICT ROLLBACK, UNIQUE(b) ON CONFLICT ABORT, UNIQUE(b, c) ON CONFLICT REPLACE)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tTEXT,\n"
"\t\"c\"\tTEXT,\n"
"\tPRIMARY KEY(\"a\") ON CONFLICT ROLLBACK,\n"
"\tUNIQUE(\"b\") ON CONFLICT ABORT,\n"
"\tUNIQUE(\"b\",\"c\") ON CONFLICT REPLACE\n"
");");

QTest::newRow("named constraints")
<< std::string("CREATE TABLE t(a INTEGER CONSTRAINT nn NOT NULL ON CONFLICT FAIL, b TEXT CONSTRAINT u UNIQUE ON CONFLICT IGNORE, CONSTRAINT pk PRIMARY KEY(a, b) ON CONFLICT REPLACE)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER CONSTRAINT \"nn\" NOT NULL ON CONFLICT FAIL,\n"
"\t\"b\"\tTEXT CONSTRAINT \"u\" UNIQUE ON CONFLICT IGNORE,\n"
"\tCONSTRAINT \"pk\" PRIMARY KEY(\"a\",\"b\") ON CONFLICT REPLACE\n"
");");

QTest::newRow("autoincrement")
<< std::string("CREATE TABLE t(a INTEGER PRIMARY KEY ON CONFLICT REPLACE AUTOINCREMENT, b TEXT)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tTEXT,\n"
"\tPRIMARY KEY(\"a\" AUTOINCREMENT) ON CONFLICT REPLACE\n"
");");

QTest::newRow("autoincrement table constraint")
<< std::string("CREATE TABLE t(a INTEGER, b TEXT, PRIMARY KEY(a AUTOINCREMENT) ON CONFLICT FAIL)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tTEXT,\n"
"\tPRIMARY KEY(\"a\" AUTOINCREMENT) ON CONFLICT FAIL\n"
");");

QTest::newRow("combined with other column constraints")
<< std::string("CREATE TABLE t(a TEXT NOT NULL ON CONFLICT REPLACE DEFAULT 'x' UNIQUE ON CONFLICT IGNORE COLLATE NOCASE)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tTEXT NOT NULL ON CONFLICT REPLACE DEFAULT 'x' UNIQUE ON CONFLICT IGNORE COLLATE NOCASE\n"
");");

QTest::newRow("without rowid")
<< std::string("CREATE TABLE t(a TEXT PRIMARY KEY ON CONFLICT IGNORE, b TEXT) WITHOUT ROWID")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tTEXT,\n"
"\t\"b\"\tTEXT,\n"
"\tPRIMARY KEY(\"a\") ON CONFLICT IGNORE\n"
") WITHOUT ROWID;");

// SQLite also accepts a conflict clause on a table CHECK constraint (it is a no-op there) and on the NULL column constraint.
// The parser must not reject these definitions because it would then no longer understand the table at all.
QTest::newRow("table check constraint")
<< std::string("CREATE TABLE t(a INTEGER, CONSTRAINT ck CHECK(a > 0) ON CONFLICT IGNORE)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\tCONSTRAINT \"ck\" CHECK(\"a\" > 0) ON CONFLICT IGNORE\n"
");");

QTest::newRow("null column constraint")
<< std::string("CREATE TABLE t(a INTEGER NULL ON CONFLICT IGNORE, b TEXT NOT NULL ON CONFLICT FAIL)")
<< std::string("CREATE TABLE \"t\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tTEXT NOT NULL ON CONFLICT FAIL\n"
");");

// Every conflict action for every kind of constraint
const char* const actions[] = {"ROLLBACK", "ABORT", "FAIL", "IGNORE", "REPLACE"};
for(const char* action : actions)
{
const std::string a = action;
const std::string head = "CREATE TABLE \"t\" (\n";

QTest::newRow(("column primary key " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER PRIMARY KEY ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER,\n\tPRIMARY KEY(\"a\") ON CONFLICT " + a + "\n);");
QTest::newRow(("column not null " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER NOT NULL ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER NOT NULL ON CONFLICT " + a + "\n);");
QTest::newRow(("column unique " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER UNIQUE ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER UNIQUE ON CONFLICT " + a + "\n);");
QTest::newRow(("table primary key " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER, PRIMARY KEY(a) ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER,\n\tPRIMARY KEY(\"a\") ON CONFLICT " + a + "\n);");
QTest::newRow(("table unique " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER, UNIQUE(a) ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER,\n\tUNIQUE(\"a\") ON CONFLICT " + a + "\n);");
QTest::newRow(("table check " + a).c_str())
<< std::string("CREATE TABLE t(a INTEGER, CHECK(a > 0) ON CONFLICT " + a + ")")
<< std::string(head + "\t\"a\"\tINTEGER,\n\tCHECK(\"a\" > 0) ON CONFLICT " + a + "\n);");
}
}

void TestTable::onConflictSurvivesTableEdits()
{
// The Edit Table dialog works on a copy of the parsed table, changes it using the functions of the model and then the table is
// recreated from the SQL statement of the changed copy (see DBBrowserDB::alterTable()). Check that editing a table does not strip
// any of its ON CONFLICT clauses.
TablePtr parsed = Table::parseSQL("CREATE TABLE t(a INTEGER PRIMARY KEY ON CONFLICT REPLACE, b TEXT NOT NULL ON CONFLICT IGNORE, "
"c TEXT UNIQUE ON CONFLICT FAIL, d TEXT, UNIQUE(c, d) ON CONFLICT ROLLBACK)");
QVERIFY(parsed->fullyParsed());
const std::string original_sql = parsed->sql();

Table edited(*parsed);

// Rename the table and a column which is part of a constraint
edited.setName("t_new");
findField(edited, "c")->setName("c2");
edited.renameKeyInAllConstraints("c", "c2");

// Modify other properties of the columns which carry a conflict clause
findField(edited, "b")->setType("VARCHAR");
findField(edited, "b")->setDefaultValue("'x'");
findField(edited, "c2")->setCheck("c2 <> ''");

// Extend and shrink the primary key again
edited.addKeyToConstraint(edited.primaryKey(), "d");
edited.removeKeyFromConstraint(edited.primaryKey(), "d");

// Add a new column and remove the column d which is also used in the unique constraint
edited.fields.emplace_back("e", "TEXT");
removeField(edited, "d");
edited.removeKeyFromAllConstraints("d");

QCOMPARE(QString::fromStdString(edited.sql()),
QString("CREATE TABLE \"t_new\" (\n"
"\t\"a\"\tINTEGER,\n"
"\t\"b\"\tVARCHAR NOT NULL ON CONFLICT IGNORE DEFAULT 'x',\n"
"\t\"c2\"\tTEXT CHECK(c2 <> '') UNIQUE ON CONFLICT FAIL,\n"
"\t\"e\"\tTEXT,\n"
"\tPRIMARY KEY(\"a\") ON CONFLICT REPLACE,\n"
"\tUNIQUE(\"c2\") ON CONFLICT ROLLBACK\n"
");"));

// The table which was edited must not have been touched
QCOMPARE(QString::fromStdString(parsed->sql()), QString::fromStdString(original_sql));

// Changing the conflict action of the primary key (what the combo box in the dialog does) must show up in the SQL
edited.primaryKey()->setConflictAction("ABORT");
QVERIFY(edited.sql().find("PRIMARY KEY(\"a\") ON CONFLICT ABORT") != std::string::npos);
}

void TestTable::parseTest()
{
QFETCH(std::string, sql);
Expand Down
5 changes: 5 additions & 0 deletions src/tests/testsqlobjects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ private slots:
void complexExpression();
void parseIdentifierWithDollar();

void onConflictAccessors();
void onConflictRoundTrip();
void onConflictRoundTrip_data();
void onConflictSurvivesTableEdits();

void parseTest();
void parseTest_data();
};
Expand Down
Loading