Enhancement Request: Option to mark all books with annotations/bookmarks

Bug #1902313 reported by ownedbycats
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fix Released

Bug Description

According to Charles Haley, making it a template function would be very slow: https://www.mobileread.com/forums/showpost.php?p=4052926&postcount=892

If that's not solvable, another potential solution would be an option somewhere to add a mark to all the books with entries in the annotation database.

Thank you!

Revision history for this message
Charles Haley (cbhaley) wrote :

Kovid, my thinking is that testing for presence of annotations requires fetching the annotations from the database. I understand why it is that way; you can't efficiently cache something that is changed by external programs, but building the result set has a cost. On the other hand, a "has_annotations" that does a "select count" would not construct the result set and probably be reasonably efficient. This could possibly be cached where the cache is invalided whenever a full annotation fetch is done, but I am not sure of the value of doing that.

What do you think?

BTW: I am finally back and can start working on my backlog.

Revision history for this message
Kovid Goyal (kovid) wrote : Re: calibre bug 1902313

@charles: Are you suggesting adding a has_annotations() method to the db
Cache object/backend? If so, that's fine by me. I dont really see how we
can cache it given that the underlying data is modified out-of-process,
one would have to set a flag in the db and check it on every cache
access. Seems overly complicated, for a relatively rare use case.

Revision history for this message
ownedbycats (ownedbycats) wrote :

Could it be possible for it to check/recalc the values whenever the annotations browser is opened? That might not work well for people who only use annotations from ebook-viewer though.

Revision history for this message
Charles Haley (cbhaley) wrote :

@ownedbycats: There are many ways to add annotations. It isn't practical to have all of them force a recalc. We would then have the situation where the answer is sometimes wrong, a situation that is a nightmare for support. If we can find a way to get a correct answer in reasonable time, where reasonable is measured in GUI performance, then I would be happy to add it.

Revision history for this message
Charles Haley (cbhaley) wrote :

@kovid, I agree that building a cache isn't the way to go. It a) is easy to get wrong, and b) might actually make performance worse because of cache management.

Instead I am thinking about adding a method "has_annotations" that produces in effect a yes/no answer on whether any format of the book has annotations. It would run the query
    'SELECT count(id) FROM annotations WHERE book=?', (book_id,)
This query is run entirely from annot_idx so in effect it is a memory lookup. No result set is generated. I ran the SQLite query analyzer which confirmed that the query uses only the index.

I just realized that this could be "annotations_count" at no extra query cost. I am not sure whether this is useful, but why not support providing the extra information.

My concern is that even with the memory-based index query the performance won't be acceptable. However, given that some people use formats_modtimes() in columns indicates that they are willing to pay what it takes to get the answer they want. It is after all their choice.

What do you think?

Revision history for this message
Kovid Goyal (kovid) wrote :

Sounds OK to me. Since its very much an opt in thing, people can decide
for themselves, if they want to pay the performance penalty by creating
such a column.

From a caching perspective, an easy way to do it, is to key it against
the last modified time of the db file. However, I dont know if it is actually
worth it, would need profiling to answer that.

Revision history for this message
Charles Haley (cbhaley) wrote :

Provided a template function annotation_count() that returns the number of annotations of any kind on any format of the book.

Changed in calibre:
status: New → Fix Committed
Revision history for this message
Kovid Goyal (kovid) wrote : Fixed in master

Fixed in branch master. The fix will be in the next release. calibre is usually released every alternate Friday.

 status fixreleased

Changed in calibre:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.