Sqlite: do not add foreign key constraints to temporary tables#736
Sqlite: do not add foreign key constraints to temporary tables#736paul-rouse merged 4 commits intoyesodweb:masterfrom
Conversation
| , scientific | ||
|
|
||
| if !flag(postgresql) && !flag(mysql) && !flag(mongodb) && !flag(zookeeper) | ||
| cpp-options: -DWITH_SQLITE -DDEBUG |
There was a problem hiding this comment.
This seems a little weird—why not have a flag for SQLite?
There was a problem hiding this comment.
It's just doing for test/main.hs what was already being done for the main test modules in the library
There was a problem hiding this comment.
Oh right, it's this way so that SQLite is the default. 👍
| , case ref of | ||
| Nothing -> "" | ||
| Just (table, _) -> " REFERENCES " <> escape table | ||
| Just (table, _) -> if noRef then "" else " REFERENCES " <> escape table |
There was a problem hiding this comment.
So the other option would be to set ref on each Column to Nothing. I kind of like that because it reduces the amount of branching. But I could also see an argument that it's confusing to modify the data of Column, which actually hasn't changed, and the Bool parameter is better because it shows that the data is being overridden. Thoughts?
There was a problem hiding this comment.
We are in the function which is being mapped over the columns, so it seemed easiest to do the work here. Does that sound reasonable?
|
Thanks @psibi ! |
3 participants
Fixes #735
@MaxGabriel, @psibi would one of you mind checking my logic, please? The issue is that migration can involve creating a temporary table, and we were adding foreign key constraints that existed in the table being copied. Sqlite doesn't handle FK constraints from temporary tables to non-temporary ones - it probably doesn't make sense to do so - and when checking is switched on, it reports them as violated.
The fix is simply to omit the FK constraint from the temporary table!