Update query fails

[Gilles Caulier-4]

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

Please find this patch for facedbschemaupdater.cpp

Is it fine to commit?

ᐧ

On Mon, Jun 6, 2016 at 12:54 AM, Swati Lodha <[hidden email]> wrote:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

The semicolon in SQL is a statement terminator. It specifies that a particular statement has ended. This allows more than one statement to be executed.

Missing ; in the queries would thus never execute that query.

ᐧ

On Tue, Jun 7, 2016 at 2:10 AM, Gilles Caulier <[hidden email]> wrote:

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

So basically this function would never be called for Face DB ?

ᐧ

On Tue, Jun 7, 2016 at 2:16 AM, Swati Lodha <[hidden email]> wrote:

The semicolon in SQL is a statement terminator. It specifies that a particular statement has ended. This allows more than one statement to be executed.

Missing ; in the queries would thus never execute that query.

ᐧ

On Tue, Jun 7, 2016 at 2:10 AM, Gilles Caulier <[hidden email]> wrote:

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

Really ? If this is true, with all ';' we can considerate that nothing can work previously.

Gilles Caulier

2016-06-06 22:46 GMT+02:00 Swati Lodha <[hidden email]>:

The semicolon in SQL is a statement terminator. It specifies that a particular statement has ended. This allows more than one statement to be executed.

Missing ; in the queries would thus never execute that query.

ᐧ

On Tue, Jun 7, 2016 at 2:10 AM, Gilles Caulier <[hidden email]> wrote:

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

Currently yes. There is a SchemaUpdater class where updating rules can be written. Check with DBCore schema updater how the rules are implemented.

Gilles Caulier

2016-06-06 22:48 GMT+02:00 Swati Lodha <[hidden email]>:

So basically this function would never be called for Face DB ?

ᐧ

On Tue, Jun 7, 2016 at 2:16 AM, Swati Lodha <[hidden email]> wrote:

The semicolon in SQL is a statement terminator. It specifies that a particular statement has ended. This allows more than one statement to be executed.

Missing ; in the queries would thus never execute that query.

ᐧ

On Tue, Jun 7, 2016 at 2:10 AM, Gilles Caulier <[hidden email]> wrote:

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Swati Lodha]

I think if ; is omitted and query compiles, it can produce unexpected results. So it helps to avoid bugs in code, that could go undetected.  

ᐧ

On Tue, Jun 7, 2016 at 2:33 AM, Gilles Caulier <[hidden email]> wrote:

Currently yes. There is a SchemaUpdater class where updating rules can be written. Check with DBCore schema updater how the rules are implemented.

Gilles Caulier

2016-06-06 22:48 GMT+02:00 Swati Lodha <[hidden email]>:

So basically this function would never be called for Face DB ?

ᐧ

On Tue, Jun 7, 2016 at 2:16 AM, Swati Lodha <[hidden email]> wrote:

The semicolon in SQL is a statement terminator. It specifies that a particular statement has ended. This allows more than one statement to be executed.

Missing ; in the queries would thus never execute that query.

ᐧ

On Tue, Jun 7, 2016 at 2:10 AM, Gilles Caulier <[hidden email]> wrote:

There is no V1 and V2 as i know. This is why code is commented (typically a copy and cast from me when i migrate libkface to digiKam core, using other DB class code as template.

Gilles Caulier

2016-06-06 22:37 GMT+02:00 Gilles Caulier <[hidden email]>:

Patch sond fine.

Q : What' s the side effect to miss ";" at end of sql statements ?

Gilles Caulier

2016-06-05 21:24 GMT+02:00 Swati Lodha <[hidden email]>:

Please find attached patch for facesdb.

Committing on git/master. Compiles without error.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 2:38 PM, Swati Lodha <[hidden email]> wrote:

I've now added  #include "digikam_debug.h"  under Local includes for qCDebug statement.

ᐧ

On Sun, Jun 5, 2016 at 2:33 PM, Gilles Caulier <[hidden email]> wrote:

Patch is fine to commit in git/master. 

qCDebug(debug namespace) is the reight way to print statements on the console. At run time, there are run to enable/disable prints.

debug namespace are declared in /core/app/utils/digikam_debug.h

Gilles Caulier

2016-06-05 10:44 GMT+02:00 Swati Lodha <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]>:

I've sent the PR. Please review.

ᐧ

On Sun, Jun 5, 2016 at 3:41 AM, Swati Lodha <[hidden email]> wrote:

Yes. 

I'll commit now. Was waiting for verification.

Thank you.

ᐧ

On Sun, Jun 5, 2016 at 3:39 AM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Final patch for coredb.cpp

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 11:44 PM, Gilles Caulier <[hidden email]> 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 <[hidden email]>:

Check this patch too.

Thank you.

ᐧ

On Fri, Jun 3, 2016 at 12:37 AM, Swati Lodha <[hidden email]> wrote:

Please check this patch.

Sorry for inconvenience.

ᐧ

On Thu, Jun 2, 2016 at 11:56 PM, Swati Lodha <[hidden email]> wrote:

Please find attached patch file for the same.

Thank you.

ᐧ

On Thu, Jun 2, 2016 at 8:57 PM, Swati Lodha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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
>     <[hidden email] <mailto:[hidden email]>>

>     >
>     > 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

>     > > [hidden email] <mailto:[hidden email]>
>     > > https://mail.kde.org/mailman/listinfo/digikam-devel
>
>     _______________________________________________
>     Digikam-devel mailing list
>     [hidden email] <mailto:[hidden email]>

>     https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
>
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

Hi,

I was also thinking along the lines of a solution "b" to have proper and clean
support.
This would require some trivial support to filter out special values at the
right places.

Using 0 as magic values usually comes handy and works fast and easy, which is
the reason why it was used, but is not 100% clean.

Marcel

> 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.
>

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

Fine for me.

SQLite didn't care about the semicolon. Other systems might be more picky.
Good to fix it.


> Final patch for coredb.cpp
>
> Thank you.

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel