search

mzabani / codd

Codd is a simple-to-use CLI tool that applies plain postgres SQL migrations atomically with strong and automatic cross-environment schema equality checks.
Other
38 stars 1 forks source link

Fix parsing when standard_conforming_strings=off #158

Open mzabani opened 1 year ago

mzabani commented 1 year ago

We should probably make our streaming parser allow its running monad to query the state of standard_conforming_strings, and postpone using that parser until we actually have connections, meaning we parse migration headers not with a streaming parser.

Using psqlscan.l from upstream would be ideal, but can come as a next step.

mzabani commented 2 months ago

While working on this I realised that because codd allows resuming failed no-txn migrations from the last failed statement, we would need to store the value of standard_conforming_strings in codd's table for that scenario, because when that happens we might skip a SET standard_conforming_strings='off' statement that is relevant to parsing the rest of the migration.

Of course, I realised the problem isn't just standard_conforming_strings. Someone might have a no-txn migration with a SET ROLE statement, and we also don't replicate that, so one could say this is really a bug that already exists in codd.

However, this is not easy to fix. There are sequences of SET ROLE statements which are valid, but one single SET ROLE last_role_set would run into an issue with privileges.

It's not clear what's best here. Maybe we should store all (or a bunch of) relevant settings to resume failed no-txn migration in addition to the statement number where it stopped, and attempt to restore that state when resuming. But that would be more work and complexity in the codebase for such an edge case that it doesn't feel worth it. At least not right now.

So maybe we do add support for standard_conforming_strings=off (and also changing it during the course of migration application), but we (still) don't restore setting state when resuming failed no-txn migrations. Thent we add sufficiently loud warnings in yellow on codd add if we detect those. That's better UX as it's more preventive.