Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MariaDB: Incorrect migration truncates descriptions in the index #2398

Closed
benjaminjonard opened this issue Jun 4, 2022 · 11 comments
Closed
Assignees
Labels
bug Something isn't working released Available in the stable release

Comments

@benjaminjonard
Copy link

benjaminjonard commented Jun 4, 2022

Hello,

I recently noticed that all descriptions on my photos where deleted (no worries, I got them back from a backup)

But with a bit of testing I could track down the issue to the release of May 17, 2022 and specifically to this migration : https://github.com/photoprism/photoprism/blob/7ba043a559a59663274d9504813cfe5981029010/internal/migrate/mysql/20220329-050000.sql

I'm using MariaDb 10.7, and if I run the following query SELECT SUBSTR(photo_description, 0, 4096) FROM photos; the result is always empty. The consequence is that the first statement in the migration file (UPDATE photos SET photo_description = SUBSTR(photo_description, 0, 4096) WHERE 1;) erases all descriptions.

When looking at MariaDB docs here https://mariadb.com/kb/en/substring/, there is written "By default, the position of the first character in the string from which the substring is to be extracted is reckoned as 1. "
Changing the query to SELECT SUBSTR(photo_description, 1, 4096) FROM photos; is working as intended. But I'm not sure it would suit everyone's configuration.

I also found this article that explains this position problem more clearly.

@lastzero
Copy link
Member

lastzero commented Jun 4, 2022

Thanks for reporting this! Taking a look at it ASAP

@lastzero lastzero self-assigned this Jun 4, 2022
@lastzero lastzero added the bug Something isn't working label Jun 4, 2022
@lastzero lastzero added the please-test Ready for acceptance test label Jun 9, 2022
@lastzero
Copy link
Member

lastzero commented Jun 9, 2022

Removed the update statements because 99% of users shouldn't have that much text in these fields and the database might truncate the content automatically anyway...

@lastzero lastzero changed the title MariaDB: migration deletes all photos descriptions MariaDB: Incorrect migration truncates photo descriptions in index Jun 13, 2022
@lastzero
Copy link
Member

I would have loved to release a fix sooner, but have been traveling all week. If no one complains, the changes from 4 days ago will be released tomorrow as part of a tiny update that otherwise just contains updated translations.

@lastzero lastzero changed the title MariaDB: Incorrect migration truncates photo descriptions in index MariaDB: Incorrect migration truncates descriptions in the index Jun 13, 2022
@lastzero lastzero added released Available in the stable release and removed please-test Ready for acceptance test labels Jun 14, 2022
@seku80
Copy link

seku80 commented Jun 15, 2022

Hello @lastzero,
First of all, I must congratulate you on the excellent project that is PhotoPrism.
Unfortunately, all the photos I have in my collection after updating from Build 220121-2b4c8e1f (docker version with MariaDB 10.6) to the latest version Build 220614-dea9ff68 have lost the description.

@lastzero
Copy link
Member

lastzero commented Jun 15, 2022

My apologies, we will adjust our testing strategy after this glitch. IMHO MariaDB should have thrown an error in this case, but we can't change it, so it's pointless to debate. Instead, simplifying backups should be prioritized on our roadmap.

We hope that more people help test and fund the project in the future, because that is the most effective way to ensure reliability and stability.

If you don't have backups, you may still find recently edited descriptions in the Sidecar YAML files. A full rescan will overwrite these, but should also restore descriptions found in the original photo metadata and XMP sidecar files.

Edit: Are you sure you didn't run any version in between or a preview build? If this still happens, that would be very strange as we removed those statements completely.

@seku80
Copy link

seku80 commented Jun 15, 2022

No worries. I have backups and have already rollback to the oldest version of Photoprism.
I've updated from the version that I've told you to the latest one, no other versions in between.

@lastzero
Copy link
Member

Glad you have backups and thanks for reporting it! I'll add more migration specific tests and look into how this can still happen. If there is one thing a database should not do, it is to unexpectedly truncate data without error or warning. So hopefully my fault or caused by the ORM auto migration.

@lastzero
Copy link
Member

lastzero commented Jun 15, 2022

The good news is that I have not been able to corrupt any photo descriptions so far by (re)running our manually created migrations, including changing the column type from TEXT to VARCHAR(4096).

  • Could you help testing by observing how the content and data type of the photos.photo_description column look before and after the upgrade?
  • Also, it would be very interesting to see if you are also experiencing the problem with a newer version of MariaDB such as 10.7 and/or 10.8?
  • Is your MariaDB service based on our official Docker Compose example? Did you change any settings and/or operate an external database instance that is shared with other applications?
  • Finally, a partial SQL dump with some sample rows from you could help with further troubleshooting if necessary.

Getting help might save me a few hours of trial and "no error" before eventually reproducing your issue locally.... may still be a bug in the ORM library or in the general application code, which is getting harder to test. I only want to do this if it is really necessary as there is more than enough other work waiting for us :) Thank you!

@lastzero lastzero added please-test Ready for acceptance test and removed released Available in the stable release labels Jun 15, 2022
@seku80
Copy link

seku80 commented Jun 17, 2022

Hello @lastzero,
Sorry for the late reply, but I got COVID and did not work yesterday...
I really don't know what happened, but this time with the same procedure, it worked perfectly.
Sorry once more for the time that you spend on this.
Thank you for all your support.

@benjaminjonard
Copy link
Author

Hello,

I tried to reproduce the problem I originally had. I updated a backup from the release March 2, 2022 to the latest one June 17, 2022 and it worked well, no descriptions were deleted.
The problem I opened the issue for seems to be resolved.

Thank you very much for your work and time 👍 PhotoPrism is really an awesome project

@lastzero
Copy link
Member

Excellent, thanks for testing it again! 💎

@graciousgrey graciousgrey added released Available in the stable release and removed please-test Ready for acceptance test labels Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

4 participants