Avoid dumping sqlite system tables into schema #735
Conversation
sqlite includes some of its system tables in the .schema output by default, which means it cannot be imported back directly into sqlite (as LoadSchema attempts to do), as you're not allowed to recreate these tables. The -nosys option can be used to prevent this behaviour. Particular issues have been seen with the sqlite_sequence table, which appears if you use an AUTOINCREMENT column as an ID for example.
|
Thanks for the issue reporting with PR! I quickly checked the option but I found that the option was added from version 3.34.0 on 2020-12-01, and I don't think all users are using this version or higher yet. Actually, the current I think using that option is very reasonable if we can make sure the version is mostly popular but could you please suggest alternative way to archive the goal? Otherwise, let's keep this PR open until we can move forward. [1] https://github.com/actions/virtual-environments#available-environments |
sio4
added
breaking change
|
If we can't use the option then we'll have to resort to parsing the schema dump and removing the tables prefixed sqlite_ manually, which doesn't sound particularly attractive to me. |
Yeah, I totally agree with your comment. It is not convenient for your condition with Let's try to find an alternative way both within the code and workaround externally. |
There was a problem hiding this comment.
Hi @mattbnz, Thank you for your patience!
I think we had enough time and now it is time to merge this PR into the main! (time flies, also Ubuntu 22.04 is now the latest image for Github Action, and I believe most developers already migrated to the newer development environment along with sqlite3 3.34.0 or later.)
I rebased the PR to the recent main branch and reviewed your change, and I would like to ask for small changes. Please let me know if you have opinions on my comment.
Thanks again!
|
please take another look - addressed those comments and rebased my fork to the current head. |
If you create a sqlite3 database with an integer ID column using AUTOINCREMENT, pop ends up with a broken migrations/schema.sql file that prevents at least tests which try to initialize databases with it, and I'm guessing perhaps other actions that use it as well from functioning correctly.
The schema is broken, because sqlite includes includes some of its system tables in the .schema output by default (in particular sqlite_sequence, which is only created in the DB when an AUTOINCREMENT column is created, which explains why this bug is probably not hit often in pop, given the default usage of UUID IDs), but you're not allowed to recreate those tables yourself. So if you dump schema with .schema and then pipe that back into sqlite (as LoadSchema attempts to do) you get an error.
The -nosys option can be used to prevent this behaviour.