[Digikam-devel] Update query fails

Swati Lodha swatilodha27 at gmail.com
Sun Jun 5 09:44:46 BST 2016


Please fnd the attached patch. Two changes made to the thumbsdb.cpp file:

1) Added ";" to the SQL queries.

2) 2 TODOs were mentioned to check the return status. So I put the query
state result in qCDebug() to check ithe query state.
Is this done correctly?

ᐧ

On Sun, Jun 5, 2016 at 12:44 PM, Swati Lodha <swatilodha27 at gmail.com> wrote:

> I'll make sure from now on, I make these fixes in master branch. And will
> also write more precise commit messages.
>
> I'll read this page and take a care in future.
>
> Thank you.
>
>>
> On Sun, Jun 5, 2016 at 11:46 AM, Gilles Caulier <caulier.gilles at gmail.com>
> wrote:
>
>> Another remark : A fix done in master can be simply synchronized in your
>> devel branch easily with git. No need to make a new dedicated commit in
>> your branch. Later, when we merge back your branch to master, this will be
>> more problematic.
>>
>> I explain well in this wiki page :
>>
>>
>> https://community.kde.org/Digikam/GSoC2014#Branches_Creation_and_Maintenance
>>
>> Take a care yo your path in git repository about your devel banch...
>>
>> Gilles Caulier
>>
>> 2016-06-05 8:11 GMT+02:00 Gilles Caulier <caulier.gilles at gmail.com>:
>>
>>> The PR ???
>>>
>>> I see your commit in your git developpement branch.
>>>
>>> I think this simple fixes can be committed directly in git master for
>>> production. There are no intrusive and do not introduce new feature.
>>>
>>> On remark about your commits : make it more atomic as possible. Your
>>> commit introduce 2 fixes : one for all missing ";" at end of SQL
>>> statements, one other to fix the mess between image and video metadata DB
>>> scan.
>>>
>>> Gilles Caulier
>>>
>>> 2016-06-05 0:46 GMT+02:00 Swati Lodha <swatilodha27 at gmail.com>:
>>>
>>>> I've sent the PR. Please review.
>>>>
>>>>>>>>
>>>> On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <swatilodha27 at gmail.com>
>>>> wrote:
>>>>
>>>>> Yes.
>>>>>
>>>>> I'll commit now. Was waiting for verification.
>>>>>
>>>>> Thank you.
>>>>>>>>>>
>>>>> On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <
>>>>> caulier.gilles at gmail.com> wrote:
>>>>>
>>>>>> The patch sound good.
>>>>>>
>>>>>> Did you have a working ssh connexion now to commit yourself in KDE
>>>>>> repository ?
>>>>>>
>>>>>> `Gilles Caulier
>>>>>>
>>>>>> 2016-06-04 20:43 GMT+02:00 Swati Lodha <swatilodha27 at gmail.com>:
>>>>>>
>>>>>>> Final patch for coredb.cpp
>>>>>>>
>>>>>>> Thank you.
>>>>>>>>>>>>>>
>>>>>>> On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <
>>>>>>> caulier.gilles at gmail.com> wrote:
>>>>>>>
>>>>>>>> This patch sound fine.
>>>>>>>>
>>>>>>>> I would to know the Marcel viewpoint here, just to be sure.
>>>>>>>>
>>>>>>>> Take a care, the patch version 1 is also included in this one.
>>>>>>>>
>>>>>>>> Gilles Caulier
>>>>>>>>
>>>>>>>> 2016-06-02 21:29 GMT+02:00 Swati Lodha <swatilodha27 at gmail.com>:
>>>>>>>>
>>>>>>>>> Check this patch too.
>>>>>>>>>
>>>>>>>>> Thank you.
>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>> On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <
>>>>>>>>> swatilodha27 at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> Please check this patch.
>>>>>>>>>>
>>>>>>>>>> Sorry for inconvenience.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>> On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <
>>>>>>>>>> swatilodha27 at gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Please find attached patch file for the same.
>>>>>>>>>>>
>>>>>>>>>>> Thank you.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>> On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <
>>>>>>>>>>> swatilodha27 at gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hello
>>>>>>>>>>>>
>>>>>>>>>>>> In coredb.cpp:4547 ";" is missing after the UPDATE query. This
>>>>>>>>>>>> change should be committed?
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Jun 1, 2016 at 6:14 PM, Richard Mortimer <
>>>>>>>>>>>> richm+digikam at oldelvet.org.uk> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> On 01/06/2016 10:12, Swati Lodha wrote:
>>>>>>>>>>>>> > I understand this now. Thank you for clarification.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > So as it mentioned around coredb.cpp:549
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     549  // We need to work around the table constraint, no
>>>>>>>>>>>>> we want to
>>>>>>>>>>>>> >     delete older stale albums with
>>>>>>>>>>>>> >     550  // the same relativePath, and adjust relativePaths
>>>>>>>>>>>>> depending on
>>>>>>>>>>>>> >     albumRoot.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > I think that this needs to be done?
>>>>>>>>>>>>>
>>>>>>>>>>>>> It is all part of the bigger picture that needs considering to
>>>>>>>>>>>>> allow
>>>>>>>>>>>>> referential integrity to be applied across the (MySQL)
>>>>>>>>>>>>> database. I don't
>>>>>>>>>>>>> have the years of experience with digikam that Marcel does but
>>>>>>>>>>>>> I think
>>>>>>>>>>>>> the following is required:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 1 - apply "ideal" referential integrity to the MySQL digikam
>>>>>>>>>>>>> schema
>>>>>>>>>>>>> (mostly done).
>>>>>>>>>>>>>
>>>>>>>>>>>>> 2 - now identify where things break down due to interaction
>>>>>>>>>>>>> with the
>>>>>>>>>>>>> outside world (file systems, other photo editors/tools).
>>>>>>>>>>>>> Generally I
>>>>>>>>>>>>> think this is easy to spot and is normally the root of an
>>>>>>>>>>>>> information
>>>>>>>>>>>>> hierarchy. (tags tree, albums, album roots spring to mind).
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think that digikam has been pretty consistent with using a
>>>>>>>>>>>>> magic "0"
>>>>>>>>>>>>> value to represent where something no-longer fits into a
>>>>>>>>>>>>> hierarchy but
>>>>>>>>>>>>> is likely to re-appear in due course.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 3 - Once identified the architectural decision is how to
>>>>>>>>>>>>> handle it in a
>>>>>>>>>>>>> database configured to enforce referential integrity. Bear in
>>>>>>>>>>>>> mind that
>>>>>>>>>>>>> this needs to continue to support SQLite too without having to
>>>>>>>>>>>>> add too
>>>>>>>>>>>>> many special cases. Note that SQLite is capable of supporting
>>>>>>>>>>>>> referential integrity so the choice may be to adopt the same
>>>>>>>>>>>>> solution
>>>>>>>>>>>>> for both.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Really the choices for handling the orphaned items are:
>>>>>>>>>>>>>
>>>>>>>>>>>>> a - disable referential integrity on those affected fields.
>>>>>>>>>>>>> That is
>>>>>>>>>>>>> almost trivial to implement but does negate some of the
>>>>>>>>>>>>> benefits.
>>>>>>>>>>>>>
>>>>>>>>>>>>> b - add "special" internal rows that can be used to collect
>>>>>>>>>>>>> these
>>>>>>>>>>>>> temporarily orphaned nodes. I know that there is one at the
>>>>>>>>>>>>> root of the
>>>>>>>>>>>>> tags tree for MySQL. Note they are a bit tricky to arrange
>>>>>>>>>>>>> because they
>>>>>>>>>>>>> really need to be created with a primary key of zero to match
>>>>>>>>>>>>> the
>>>>>>>>>>>>> existing behaviour of putting zero in there.
>>>>>>>>>>>>>
>>>>>>>>>>>>> c - change to use null instead of a magic placeholder value.
>>>>>>>>>>>>> That is
>>>>>>>>>>>>> probably a purer solution but does create differences between
>>>>>>>>>>>>> SQLite and
>>>>>>>>>>>>> MySQL.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'd probably suggest that option b is the right one to
>>>>>>>>>>>>> minimise chances
>>>>>>>>>>>>> of short/medium term breakage but would be interested to know
>>>>>>>>>>>>> what
>>>>>>>>>>>>> others think.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>
>>>>>>>>>>>>> Richard
>>>>>>>>>>>>>
>>>>>>>>>>>>> P.S. even if b & c are chosen there are some potential issues
>>>>>>>>>>>>> that may
>>>>>>>>>>>>> occur if multiple orphaned tags, folders, images have the same
>>>>>>>>>>>>> name.
>>>>>>>>>>>>> That will trip up with existing "unique" constraints on the
>>>>>>>>>>>>> database. I
>>>>>>>>>>>>> suspect that this should just be left as "good enough" for now.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > Regards
>>>>>>>>>>>>> > ᐧ
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > On Wed, Jun 1, 2016 at 2:28 PM, Marcel Wiesweg <
>>>>>>>>>>>>> marcel.wiesweg at gmx.de
>>>>>>>>>>>>> > <mailto:marcel.wiesweg at gmx.de>> wrote:
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     Have a look at the (rather complex) CollectionScanner.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     The problem is the nature of our scan, which will always
>>>>>>>>>>>>> run in all
>>>>>>>>>>>>> >     kinds of
>>>>>>>>>>>>> >     race conditions with the file system.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     When a folder is moved, we may notice that it is removed
>>>>>>>>>>>>> at the previous
>>>>>>>>>>>>> >     location. Later, we see it appear at the new location.
>>>>>>>>>>>>> It would be
>>>>>>>>>>>>> >     unfortunate
>>>>>>>>>>>>> >     to have completely deleted all album metadata in the
>>>>>>>>>>>>> meantime.
>>>>>>>>>>>>> >     Therefore, it
>>>>>>>>>>>>> >     is made a stale album, which can be resurrected later if
>>>>>>>>>>>>> a new place
>>>>>>>>>>>>> >     can be
>>>>>>>>>>>>> >     identified.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     This is done in a similar way for images. Here, we can
>>>>>>>>>>>>> identify them by
>>>>>>>>>>>>> >     content, so the storage of "removed" images is even more
>>>>>>>>>>>>> extensively
>>>>>>>>>>>>> >     done.
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     > This query is executed in
>>>>>>>>>>>>> core/libs/database/coredb.cpp:576 in the function
>>>>>>>>>>>>> >     > to create stale Albums, that is why the albumRoot has
>>>>>>>>>>>>> been set to 0.
>>>>>>>>>>>>> >     >
>>>>>>>>>>>>> >     > I would like to ask why is there a need to create the
>>>>>>>>>>>>> stale Albums?
>>>>>>>>>>>>> >     >
>>>>>>>>>>>>> >     > Regards
>>>>>>>>>>>>> >     > ᐧ
>>>>>>>>>>>>> >     >
>>>>>>>>>>>>> >     > On Wed, Jun 1, 2016 at 1:19 AM, Marcel Wiesweg
>>>>>>>>>>>>> >     <marcel.wiesweg at gmx.de <mailto:marcel.wiesweg at gmx.de>>
>>>>>>>>>>>>> >     >
>>>>>>>>>>>>> >     > wrote:
>>>>>>>>>>>>> >     > > Please check the context where it is called, but I
>>>>>>>>>>>>> am quite sure
>>>>>>>>>>>>> >     that
>>>>>>>>>>>>> >     > > there is
>>>>>>>>>>>>> >     > > a special meaning in setting album root to 0.
>>>>>>>>>>>>> >     > > I'm sure there is a similar case with deleted images
>>>>>>>>>>>>> where album
>>>>>>>>>>>>> >     is set to
>>>>>>>>>>>>> >     > > 0,
>>>>>>>>>>>>> >     > > but the entry is preserved in case the image
>>>>>>>>>>>>> reappears at a
>>>>>>>>>>>>> >     different
>>>>>>>>>>>>> >     > > place
>>>>>>>>>>>>> >     > > (moving files where the removal is noticed first)
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > Marcel
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > > Hello.
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > I was facing this error from past few days:
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > MariaDB [digikam]> update Albums set albumRoot=0
>>>>>>>>>>>>> where id=10;
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > > ERROR 1452 (23000): Cannot add or update a child
>>>>>>>>>>>>> row: a
>>>>>>>>>>>>> >     foreign key
>>>>>>>>>>>>> >     > > > > constraint fails (`digikam`.`Albums`, CONSTRAINT
>>>>>>>>>>>>> >     `Albums_AlbumRoot
>>>>>>>>>>>>> >     > > > > s` FOREIGN KEY (`albumRoot`) REFERENCES
>>>>>>>>>>>>> `AlbumRoots` (`id`)
>>>>>>>>>>>>> >     ON DELETE
>>>>>>>>>>>>> >     > > > > CASCADE ON UPDATE CASCADE)
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > I figured out that as 'id' is AlbumRoots table is
>>>>>>>>>>>>> not 0. So
>>>>>>>>>>>>> >     'albumRoot'
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > in
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > > Images table can't be set to 0. This is I think
>>>>>>>>>>>>> the possible
>>>>>>>>>>>>> >     reason for
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > FK
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > > constraint failing.
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > I used this statement instead & it worked:
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > MariaDB [digikam]> update Albums set albumRoot=1
>>>>>>>>>>>>> where id=10;
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > > Query OK, 0 rows affected (0.06 sec)
>>>>>>>>>>>>> >     > > > > Rows matched: 1  Changed: 0  Warnings: 0
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > Please look into this.
>>>>>>>>>>>>> >     > > >
>>>>>>>>>>>>> >     > > > Regards
>>>>>>>>>>>>> >     > > > ᐧ
>>>>>>>>>>>>> >     > >
>>>>>>>>>>>>> >     > > _______________________________________________
>>>>>>>>>>>>> >     > > Digikam-devel mailing list
>>>>>>>>>>>>> >     > > Digikam-devel at kde.org <mailto:Digikam-devel at kde.org>
>>>>>>>>>>>>> >     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >     _______________________________________________
>>>>>>>>>>>>> >     Digikam-devel mailing list
>>>>>>>>>>>>> >     Digikam-devel at kde.org <mailto:Digikam-devel at kde.org>
>>>>>>>>>>>>> >     https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> >
>>>>>>>>>>>>> > _______________________________________________
>>>>>>>>>>>>> > Digikam-devel mailing list
>>>>>>>>>>>>> > Digikam-devel at kde.org
>>>>>>>>>>>>> > https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>>>>>> >
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> Digikam-devel mailing list
>>>>>>>>>>>>> Digikam-devel at kde.org
>>>>>>>>>>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> Digikam-devel mailing list
>>>>>>>>> Digikam-devel at kde.org
>>>>>>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Digikam-devel mailing list
>>>>>>>> Digikam-devel at kde.org
>>>>>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Digikam-devel mailing list
>>>>>>> Digikam-devel at kde.org
>>>>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Digikam-devel mailing list
>>>>>> Digikam-devel at kde.org
>>>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>>>
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> Digikam-devel mailing list
>>>> Digikam-devel at kde.org
>>>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>>>
>>>>
>>>
>>
>> _______________________________________________
>> Digikam-devel mailing list
>> Digikam-devel at kde.org
>> https://mail.kde.org/mailman/listinfo/digikam-devel
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/digikam-devel/attachments/20160605/005b5083/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: thumbsdb.patch
Type: text/x-patch
Size: 3153 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/digikam-devel/attachments/20160605/005b5083/attachment.bin>


More information about the Digikam-devel mailing list