Review Request: use sqlite for storage

[Marco Martin]

> On 2010-10-01 17:13:46, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 41
> > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line41>
> >
> >     instead of an incrementing connectionId, you could also use the value of the this pointer? one less static int kicking around.

uhm, i tought using a pointer value was even uglier, but ok.
another thing that i tought seeing it. is really necessary having a different connection per job, since the query execution is afaik syncronous?
even tought aboout a little singleton that holds a single QSqlDatabase...


> On 2010-10-01 17:13:46, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/private/storage.cpp, lines 46-49
> > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line46>
> >
> >     destination() is no longer used; with one db file used for all the storage, we end up with all the data in one db with no separation.
> >     
> >     it would make sense to me here to create a table for each destination().
> >     
> >     on applet destruction, it could drop its table (if any).
> >     
> >     on applet export, it could pull its table over to an exported db. (i imagine that Corona would use this in its export method).

or, destination could be a field (with key,destination being the primary key) would be ugly, but easier to expire old entries
but yeah, i like more the different tbles approach in general
also, if this is going to be used by applets, the source field doesn't make sense here...


> On 2010-10-01 17:13:46, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 66
> > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line66>
> >
> >     ah, sql databases.
> >     
> >     this query will result in multiple rows with the same key, and when requested back you'll end up with a random row.
> >     
> >     to fix this, first delete the old row. or check if the old row exists and do an update instead of an insert. and no, there is no "update or insert" in sql. that's what sorted procedures are for ;P

uhm, i think even worse, since key is primary the inseret should just silently fail, so the old stale entry wins
i think i'll just try to delete the line before.


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5506/#review7926
-----------------------------------------------------------


On 2010-10-01 15:05:11, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5506/
> -----------------------------------------------------------
> 
> (Updated 2010-10-01 15:05:11)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> looking at how slow kconfig as, this makes storage use sqlite (and makes some methods private, before it's too late)
> it can still use some improvements but it's basically working.
> two main concerns are:
> - is acceptable/safe to link to QtSql and assume the sqlite driver is present?
> - i would still see this as a fallback for when an akonadi version is not present (being in another process should slowdown the gui a bit less, but i could not want it in some mobile profiles)
> 
> the akonadi version is in the usual status "almost working with developer disappeared" :p
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1179394 
>   /trunk/KDE/kdelibs/plasma/datacontainer.h 1179394 
>   /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1179394 
>   /trunk/KDE/kdelibs/plasma/dataengine.cpp 1179394 
>   /trunk/KDE/kdelibs/plasma/private/datacontainer_p.h 1179394 
>   /trunk/KDE/kdelibs/plasma/private/storage.cpp 1179394 
>   /trunk/KDE/kdelibs/plasma/private/storage_p.h 1179394 
> 
> Diff: http://svn.reviewboard.kde.org/r/5506/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20101001/48216199/attachment.htm