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

SQLite errors not catched #698

Closed
dominiczedler opened this issue May 15, 2023 · 10 comments
Closed

SQLite errors not catched #698

dominiczedler opened this issue May 15, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@dominiczedler
Copy link
Collaborator

No description provided.

@dominiczedler
Copy link
Collaborator Author

@Atemu You have something big in one of your recipes, please look if you have a large image or sth like that in the preparation description of a recipe (this would explain the exception). I will implement a catch for SQLite errors, that is indeed still missing.

@Atemu
Copy link

Atemu commented May 15, 2023

Continuation from #652 (comment)

@Atemu
Copy link

Atemu commented May 15, 2023

We do indeed have a recipe with an image in its preparation description and the HTML is therefore little more than a huge block of base64. 3MiB of it.

That should be perfectly valid though since the web UI allows you to embed images into recipe descriptions like this.

@Atemu
Copy link

Atemu commented May 15, 2023

Deleting said recipe does indeed fix the issue after clearing the app's data.

@dominiczedler dominiczedler added the enhancement New feature or request label May 15, 2023
@dominiczedler
Copy link
Collaborator Author

Thanks for the info, that was helpful.

That should be perfectly valid though since the web UI allows you to embed images into recipe descriptions like this.

No, this is not perfectly valid. Android doesn't work like the webserver does. :)

Chat with ChatGPT:

I use Room in my Android app to store data in and load data from a database. One table contains recipes, where one column contains the preparation instructions, which can get quite large due to photos in base64 format. Some users report an error in this context when they have too large photos in the instruction:

android.database.sqlite.SQLiteBlobTooBigException: Row too big to fit into CursorWindow requiredPos=0, totalRows=8

Can Room's CursorWindow size be increased for this?

The CursorWindow size is a fixed limit in the Android system and cannot be changed. The maximum size of a CursorWindow is 2MB. If a single row or a set of rows in a query exceeds this limit, you will receive the SQLiteBlobTooBigException error.
To circumvent this problem, you can do the following:

  1. Reduce the size of the images: When you store images in your database, you should compress them or reduce their resolution before storing. This way, you can reduce the size of the images and prevent them from exceeding the CursorWindow size. In general, it's not a good practice to store large images directly in an SQLite database.
  2. Store the images elsewhere: Instead of storing the images directly in the database, you could store them in internal storage or on the SD card and only store the path to the image in the database. This would significantly reduce the space that the images take up in the database.
  3. Split the data across multiple rows: If the data for a row is too large, you could split it across multiple rows. For example, you could store each instruction for a recipe in a separate row, instead of storing all instructions in a single row.

It's important to note that storing large amounts of data in an SQLite database, especially large BLOBs like images, can lead to performance issues. It is often better to store this data elsewhere and only keep references to this data in the database.

I have no control over the table structure, so you have to reduce the size of your images. Generally it is not the best practice of the grocy server to store images in the text, but I understand its developer because other ways would be harder to implement.

@Atemu
Copy link

Atemu commented May 15, 2023

1. Is not an option since images can have arbitrary sizes and current standards will put most images past the 2M mark.
2. Sounds like a good idea. You could simply mirror the Grocy server's path structure: /recipe/{id}/preparation.html. I don't really think we need a database to hold most of this information to begin with since we only query for titles for search.
3. Sounds hairy to implement and generally unclean.

Related question: Why does grocy-android need to store recipes locally at all? I could understand keeping recipes cached after loading them once to speed up access the next time but why do they need to be fetched and persisted unconditionally?

Also, it seems ChatGPT is being confidently incorrect (as it does); CursorWindow size can be increased on newer API levels: https://medium.com/androiddevelopers/large-database-queries-on-android-cb043ae626e8 (bottom)

@berrnd
Copy link

berrnd commented May 15, 2023

Generally it is not the best practice of the grocy server to store images in the text

Just wanted to add that here for completeness: Recipe pictures (and product pictures, equipment files and Userfield-files) are not stored in the database, but on disk in <GROCY_DATAPATH>/storage.

That images (and much more) are also possible to be used in any WYSIWYG editor (like for the recipe preparation text) is a feature of the used editor component Summernote. By default all embedded files are converted to "BASE64 data URIs" which can lead to very much text data at the end. That's not a general problem for SQLite (which grocy also uses server-side).

Theoretically this could be changed (doc ref) (related to the web-frontend) and if that's a big problem here, I can have a look (shouldn't be too much todo, but don't know if that helps at the end when adding images from within grocy-android).

@VNRARA
Copy link

VNRARA commented May 15, 2023

@dominiczedler @patzly

Going back to the title of this issue: The app still shows tons of ambiguous network errors when adding a existing or a conflicting conversion or barcode. Does this issue cover that? If not would it be handy to make a list somewhere with all vague errors that aren't handled/shown properly yet to avoid separate issues, put them here as I come across them or do you guess prefer one new issue per error.

@Atemu
Copy link

Atemu commented May 16, 2023

@VNRARA that's a different topic. I'd appreciate you open a separate issue on that.

@dominiczedler
Copy link
Collaborator Author

@berrnd Thanks for your comment! For now it seems possible to simply increase the limit in our app. So it is not necessary to make things more complex with Summernote (I'm currently using Summernote too in our app). :)

@Atemu

Related question: Why does grocy-android need to store recipes locally at all? I could understand keeping recipes cached after loading them once to speed up access the next time but why do they need to be fetched and persisted unconditionally?

We want the possibility in our app to view pages like lists also while being offline. I think there is no difference between caching and storing.

Also, it seems ChatGPT is being confidently incorrect (as it does); CursorWindow size can be increased on newer API levels

Yes, you're right. I've added some lines which extend the limit to 10 MB (I don't know how much impact on speed this has but I would not make it too large).

Generally, error catching and details view of errors will be strongly improved in the upcoming release. If embedded pictures still exceed the limit in summary of 10 MB, an error while loading will be displayed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants