qpdfview team mailing list archive

[Adam Reichold <adam.reichold@xxxxxxxxxxx>]

Hello again,

Am 20.09.2014 um 16:34 schrieb Seyyed Razi Alavizadeh:
> Hello Adam,
>  
> 
>         > 2- I want to save firstPage state to database (indeed I wrote the patch) but I don't understand the following part of preparePerFileSettings_v2()?
>         >
>         >     if(Settings::instance()->mainWindow().restorePerFileSettings())
>         >     {
>         >         query.exec("DELETE FROM perfilesettings_v2 WHERE filePath IN (SELECT filePath FROM perfilesettings_v2 ORDER BY lastUsed DESC LIMIT -1 OFFSET 1000)");
>         >     }
>         >     else
>         >     {
>         >         query.exec("DELETE FROM perfilesettings_v2");
>         >     }
> 
>     It is our probably not very elegant way to keep only the 1000 least
>     recently used entries of the per-file settings table (or completely
>     clear it if the user disabled the option). The number 1000 is pretty
>     arbitrary and was chosen conservatively and could propbably be improved.
> 
>  
> Then is it a typo? It should be "perfilesettings_v1"?
> "perfilesettings_v2" is a fresh table, If I'm right then what I did in
> the following patch [1] seems better.

No, not so much a typo as a proper bug which was introduced when this
code was refactored. The limiting should happen every time the database
is loaded, not just when the table is created and hence needs to be
moved back to constructor and does not need to happen during the
migration phase. I'll fix this when merging your branch. (Which will
probably mean two weeks for 0.4.12beta1 instead of one for proper
stabilization.)

>     More elegant SQL to keep only the last N entries would of course also be
>     welcome.
> 
> Maybe this thread [2] is what you want? also do you know if the
> message's comment [3] about is correct or not? 
> 
> [1] https://code.launchpad.net/~srazi/qpdfview/store-first-page-per-file
> [2] http://stackoverflow.com/a/18677756
> [3] http://stackoverflow.com/a/6745571

Using the "NOT IN" instead of the "IN" predicate from [3] will slighty
simplify the code but is exactly the same conceptually. [2] is IMHO much
more complicated than is useful in our case.

And if by comment you relate to the use of "VACUUM", I don't think our
usage pattern warrants a more aggressive method than the default usage
of SQLite's internal free list which will just recycle unused pages.
After all, the complete database tends to be in the sub-megabyte range
AFAIK.

> Best Regards,
> Razi.

Best regards, Adam.