Skip to content

Conversation

@python357-1
Copy link
Collaborator

@python357-1 python357-1 commented Dec 17, 2024

This PR fixes #644 by having make_tables check the current autoincrement value (what the next inserted row would have as its id) on the tags table. Previously, an INSERT and a DELETE were always being run when a library was opened, causing the file change counter to increase

close #644
close #646

@python357-1 python357-1 added Type: Bug Something isn't working as intended Type: Tests Tests or testing related Status: Review Needed A review of this is needed labels Dec 17, 2024
@Computerdores
Copy link
Collaborator

Only running the DELETE and INSERT when they are necessary is definitely the better solution!
However could you explain why you did it this way? My intuition would have been to check whether the schema already matches and to DELETE and INSERT if not.

@python357-1
Copy link
Collaborator Author

I don't know how to check if the schema is the same to do this - even the sqldiff tool didn't detect this. The reason we are doing the insert and delete is to make sure the next auto increment ID is at least 1000. This implementation allows it to skip updating the database if the next ID is at least 1000.

Copy link
Collaborator

@Computerdores Computerdores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the issue for me on Win10 and I don't see anything wrong with the code either (no idea whether using sessionmaker is better here, but I see nothing wrong with it)

@python357-1
Copy link
Collaborator Author

yeah i actually just fixed that, realized there was no need for that to be there lol

@Computerdores Computerdores added the Priority: High An important issue requiring attention label Dec 17, 2024
Copy link
Member

@CyanVoxel CyanVoxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this fixes the modification issues without breaking the auto-increment values 👍

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Dec 20, 2024
@CyanVoxel CyanVoxel added this to the SQL Parity milestone Dec 20, 2024
@CyanVoxel CyanVoxel merged commit 24fa76e into TagStudioDev:main Dec 20, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: High An important issue requiring attention Type: Bug Something isn't working as intended Type: Tests Tests or testing related

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[Bug]: Pytest modifies and updates search_library sqlite file

3 participants