shiori
shiori copied to clipboard
go-shiori
Sql
Hello,
Two commit to solve issues encountered using shiori with postgresql.
The serial auto increment was not working, and made a constraint violation for all bookmarks insertions after a delete. I also improved auto tag bookmark relation deletion with an on cascade delete.
The second commit is the removal of all invalid runes for utf8 before inserting in the db as it caused an error with pg.
Note that it might mask a bug in another part of the programs on the validity of the fetched bookmark content.
Hi @ynsta, thanks for the PR.
I'm thinking that the UTF-8 fix should probably be applied globally, i.e. we shouldn't be putting screwed-up data in any database, even if MySQL and SQLite don't complain about it. What do you think?
Regarding the ID fix, I think we should also be using autoincrement (and cascading deletes) across all 3 databases, too.
Rather than adding a fix for Postgres and turning CreateNewID() into a noop, I'm thinking that CreateNewID() should be scrapped completely. The way I'd normally do it is to pass in pointers to Bookmark structs, and SaveBookmarks() would populate the ID field of any new ones.
In addition, I'd like to add migration support before making any further changes to DB schemas. As best as I can tell from reading the source code, your changes won't work well on existing installations because they won't have the updated DB schema.
What do you think?
I not yet looked at your changes is this PR useful anymore ?
I haven't changed the database code yet. Please see my comments above.
@deanishe @ynsta sounds like a smart move. The unicode fix is definitely cross db sensible, and the serial/auto-increment makes sense for all systems at this point. what will it take to make that happen?
If needed I can look at it again but don't know yet if @deanishe still want to rework the sql.
Hey- just working on an import with pg and finding that the constraint and tag aliggnment is messed up. this definitely needs to get fixed. can we do what we can to move it forward?
So. I would like to move this forward. The CreateNewID will be tackled once #271 gets picked, but I don't know what's hapepening with the runes or utf-8. Can someone guide me through the problem so I can reproduce it?
@fmartingr i'm pretty slammed with work work right now, but once it lets up i'll try and jump back to this, as i've got way way too many documents full of links i want to centralize :P
@imajes Of course! No problem, I just want to understand what the problem is because I will be changuing things related to the database engines in the following week (hopefully) so I would want to know what I have to take into consideration.