[digikam] [Bug 355831] New: MySQL Schema Improvements

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Bug ID: 355831
Summary: MySQL Schema Improvements
Product: digikam
Version: 5.0.0
Platform: Compiled Sources
OS: Linux
Status: UNCONFIRMED
Severity: wishlist
Priority: NOR
Component: Database-Schema
Assignee: digikam-***@kde.org
Reporter: richm+***@oldelvet.org.uk

Schema improvements to better support MySQL.


Rewrite the MySQL database schema to remove the need for SUPER privileges when
creating/amending the digikam database.

Use foreign key referential integrity to achieve the behaviour of the triggers.
Specifically use ON DELETE to cascade deletion of a Tag, Image, Album or
AlbumRoot any items that depend upon it.

This change does make the database less tolerant of bad data that references
non-existant entries. But that can be argued to be a good thing because it
causes a fast fail in the case of bugs/issues that would have otherwise caused
silent data loss.

The schema changes are not fully completed yet and is intended as a proof of
concept patch series. Comments most welcome!

Notes:

Testing performed using the following MySQL setup.

GRANT USAGE ON *.* TO 'digiuser'@'localhost' IDENTIFIED BY 'abc123';
GRANT ALL PRIVILEGES ON `digikam`.* TO 'digiuser'@'localhost';
CREATE DATABASE digikam;

The conversion code was failing to catch SQL errors in the migration process. I
corrected this by returning an error if a query fails but it is not clear if
this is the intended behaviour and it maybe that this will uncover other issues
both in SQLite and MySQL.

The Albums table has an icon field that causes a circular reference to the
Images table. This needs special handling both during table deletion and data
migration. I put a hack in place for the table deletion case but did not
address the migration where images will initially need creating without the
icon being set and then the icon values need copying over once the Images have
been migrated.

The migration will fail if any orphan records exist that do not reference an
existing AlbumRoot, Album, Image or Tag. Normally that would be due to some
previous uncaught error due to failed creation or partial deletion.

I did notice that in my newly created SQLite database with only a few test
images the Tags "icon" field was populated with a zero instead of a null. This
causes the migration to fail. I worked around this by executing the following
against the SQLite database.

update Tags set icon = null where icon = 0;

The thumbnail and face schema have not been fully examined/converted yet. The
triggers have been disabled to allow migration testing.

Similarly I have not looked at database upgrade from the previous version of
the MySQL schema. There is at least one user (myself) who has been using MySQL
so having a proper upgrade to add the referential integrity would seem to be
needed in a final solution.


Reproducible: Always

Steps to Reproduce:
1. Proof of concept patch series

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #1 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95686
--> https://bugs.kde.org/attachment.cgi?id=95686&action=edit
[PATCH 0/5] MySQL schema improvements

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #2 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95687
--> https://bugs.kde.org/attachment.cgi?id=95687&action=edit
[PATCH 1/5] Ensure that an error is returned if the query fails.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #3 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95688
--> https://bugs.kde.org/attachment.cgi?id=95688&action=edit
[PATCH 2/5] Delete tables in reverse creation order.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #4 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95689
--> https://bugs.kde.org/attachment.cgi?id=95689&action=edit
[PATCH 3/5] Ensure that data is migrated in line with foreign key

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #5 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95690
--> https://bugs.kde.org/attachment.cgi?id=95690&action=edit
[PATCH 4/5] Remove trigger dependency for MySQL.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #6 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95691
--> https://bugs.kde.org/attachment.cgi?id=95691&action=edit
[PATCH 5/5] Temporary workaround to break MySQL references loop with the Albums
icon entry.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #7 from ***@gmail.com ---
Git commit 38cb75f52bc7f602fe75d188106ffb8f86a4d568 by Gilles Caulier.
Committed on 24/11/2015 at 12:02.
Pushed by cgilles into branch 'master'.

apply patch #95687 to return an error if SQL query fails.

M +4 -0 libs/database/engine/dbenginebackend.cpp

http://commits.kde.org/digikam/38cb75f52bc7f602fe75d188106ffb8f86a4d568

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #8 from ***@gmail.com ---
Git commit 4853eedeac43d4a7c59942e40153482f7732e150 by Gilles Caulier.
Committed on 24/11/2015 at 12:34.
Pushed by cgilles into branch 'master'.

apply patch #95689 to be able to use of referential integrity checks in the
database schema

M +2 -2 libs/database/coredb/coredbcopymanager.cpp

http://commits.kde.org/digikam/4853eedeac43d4a7c59942e40153482f7732e150

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com
Attachment #95687|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95689|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95686|0 |1
is obsolete| |

--- Comment #9 from ***@gmail.com ---
Comment on attachment 95686
--> https://bugs.kde.org/attachment.cgi?id=95686
[PATCH 0/5] MySQL schema improvements

From 8cfb15da5f4de65a734aa864c68871822e144de1 Mon Sep 17 00:00:00 2001
Date: Tue, 24 Nov 2015 08:56:22 +0000
Subject: [PATCH 0/5] MySQL schema improvements
Rewrite the MySQL database schema to remove the need for SUPER privileges
when creating/amending the digikam database.
Use foreign key referential integrity to achieve the behaviour of the
triggers. Specifically use ON DELETE to cascade deletion of a Tag, Image,
Album or AlbumRoot any items that depend upon it.
This change does make the database less tolerant of bad data that references
non-existant entries. But that can be argued to be a good thing because
it causes a fast fail in the case of bugs/issues that would have otherwise
caused silent data loss.
The schema changes are not fully completed yet and is intended as a proof of
concept patch series. Comments most welcome!
Testing performed using the following MySQL setup.
CREATE DATABASE digikam;
The conversion code was failing to catch SQL errors in the migration
process. I corrected this by returning an error if a query fails but it is
not clear if this is the intended behaviour and it maybe that this will
uncover other issues both in SQLite and MySQL.
The Albums table has an icon field that causes a circular reference to
the Images table. This needs special handling both during table deletion
and data migration. I put a hack in place for the table deletion case but
did not address the migration where images will initially need creating
without the icon being set and then the icon values need copying over
once the Images have been migrated.
The migration will fail if any orphan records exist that do not reference an
existing AlbumRoot, Album, Image or Tag. Normally that would be due to
some previous uncaught error due to failed creation or partial deletion.
I did notice that in my newly created SQLite database with only a few test
images the Tags "icon" field was populated with a zero instead of a null.
This causes the migration to fail. I worked around this by executing the
following against the SQLite database.
update Tags set icon = null where icon = 0;
The thumbnail and face schema have not been fully examined/converted yet.
The triggers have been disabled to allow migration testing.
Similarly I have not looked at database upgrade from the previous version
of the MySQL schema. There is at least one user (myself) who has been
using MySQL so having a proper upgrade to add the referential integrity
would seem to be needed in a final solution.
Ensure that an error is returned if the query fails.
Delete tables in reverse creation order.
Ensure that data is migrated in line with foreign key dependencies.
Remove trigger dependency for MySQL.
Temporary workaround to break MySQL references loop with the Albums
icon entry.
data/database/dbconfig.xml.cmake.in | 82 +++++++++++++++---------------
libs/database/coredb/coredbcopymanager.cpp | 9 ++--
libs/database/engine/dbenginebackend.cpp | 2 +
3 files changed, 48 insertions(+), 45 deletions(-)
--
2.5.0

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #10 from ***@gmail.com ---
Git commit e9cad0b63d2d677d58ab55e8208af591f9cd1298 by Gilles Caulier.
Committed on 24/11/2015 at 12:46.
Pushed by cgilles into branch 'master'.

apply patch #95688 to delete tables in reverse creation order.

M +1 -1 libs/database/coredb/coredbcopymanager.cpp

http://commits.kde.org/digikam/e9cad0b63d2d677d58ab55e8208af591f9cd1298

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95688|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #11 from ***@gmail.com ---
Richard,

About patch 4/5, i need to test. A lots of code SQL has changed.
About patch 5/5, if you have a better solution to patch SQL statements, let's
me hear.

Gilles

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #12 from ***@gmail.com ---
About 5/5 : why CreateFaceTriggers is commented now ?

About database init, before we use :

CREATE DATABASE digikam; GRANT ALL PRIVILEGES ON digikam.* TO
'root'@'localhost' IDENTIFIED BY 'password'; FLUSH PRIVILEGES;

now, you use :

GRANT USAGE ON *.* TO 'digiuser'@'localhost' IDENTIFIED BY 'abc123';
GRANT ALL PRIVILEGES ON `digikam`.* TO 'digiuser'@'localhost';
CREATE DATABASE digikam;

=> You create database at end. Why ?
=> you grant usage to dedicated user. Why ?
=> you grant privilege to this dedicated user too. Why ?
=> you do not flush privilege. Why ?

Is you rules are to create a database only usable by a dedicated user, and not
root ? This will permit to share the same database with more than one users at
the same time ?

Gilles Caulier

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #13 from ***@gmail.com ---
If the schema is updated, the schema version must be patched in
*schemaupdater.cpp classes. New rules to update schema with relevant DB action
must be create to be able to migrate old schema to new one.

Gilles Caulier

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #14 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
CreateFaceTriggers is commented because it is still a TODO change to replace it
with references. I just commented it to stop any chance of an error with the
reduced privileges whilst testing. I will replace this when I produce another
version of that patch.

=> You create database at end. Why ?

It will work at the start or end. It is just habit that I create the database
after setting up permissions.

=> you grant usage to dedicated user. Why ?

The usage line sets up the password for the dedicated user. The password is not
tied to a specific database so it is clearer to set that up separately from
giving access to a database. Note that "USAGE" infers no privileges to a user
it just provides a convenient way to change user account settings.
See http://dev.mysql.com/doc/refman/5.5/en/privileges-provided.html#priv_usage

=> you grant privilege to this dedicated user too. Why ?

The second grant gives access to the specific database for that user. That is
done at database level so it give all database level privileges (create
database, table etc.) without giving database server administration level
privileges.

=> you do not flush privilege. Why ?

I do not think that flush is needed if you use the GRANT statement method to
setup privileges. But it does not hurt to include it.
See http://dev.mysql.com/doc/refman/5.5/en/privilege-changes.html

Use of a dedicated user and not root is for security reasons. The root user
tends to have full database administration privileges and that is not a good
thing to encourage.

If you want to give multiple users access to the same database just issue the
same grant commands for the additional users. You could also use a more tightly
controlled set of privileges for these different users.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #15 from ***@gmail.com ---
About patch 5/5 : it's Sqlite or/and Mysql relevant fix ?

Don't forget that SQlite schema can be considerate as stable. So take a care
about change here. An update version must be created with a db action dedicated
to migrate to new schema.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #16 from ***@gmail.com ---
You explanation in comment #14 are fine for me. It's definitively the good way
to go.

About Mysql, as the support have been always considerated as experimental, if
we change a lots the DB schema, the best way will be to set older one as
obsolete and incompatible with new one. Everywhere, the users must be warned
about the major changes introduced to the new schema and no migration rules
cannot be written easily.

Typically, in this case, the user must use XMP sidecar solution over collection
for ex to save DK metadata, and create a new mysql DB with a fresh parsing.

This will reduce the complexity in SQL statements rules.

What do you think about ?

Gilles Caulier

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Richard Mortimer <richm+***@oldelvet.org.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95690|0 |1
is obsolete| |
Attachment #95691|0 |1
is obsolete| |

--- Comment #17 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95731
--> https://bugs.kde.org/attachment.cgi?id=95731&action=edit
Updated trigger removal patch for MySQL databases

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #18 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
I have not had chance to look at MySQL upgrade procedure yet - although given
the changes made so far it should just be adding foreign keys so assuming that
the data is not broken due to orphaned records then that should be OK.

Similarly I have not had chance to look at the tags issue on MySQL yet.

A couple of other things I have noticed:

1 - Each of the image, thumbnails and face recognition databases all have a
table called Settings. I can see this causing trouble with all three merged
into a single database. At a very minimum the migration needs to take care to
track this and migrating back to separate databases would be even harder. Is it
going to be feasible to rename the thumbnails and face recognition tables to
something like SettingsThumb and SettingsFace or would that cause too much
trouble with SQLite support?

2 - Currently migration does not seem to handle the faces and thumbnails
databases. Presumably this is planned but just not done yet.

3 - the logic surrounding m_isStopProcessing in coredbcopymanager.cpp looks to
be wrong in places. The loops do a OR operation, e.g. "m_isStopProcessing || i

=0", but not all of them test m_isStopProcessing to terminate the migration if

that flag is set.

Also using that way to terminate the loop the migration the code must be
careful not to access the loop counter "i" before terminating because it could
cause an overflow of the tables array if the termination is received during the
processing of the last table being iterated.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #19 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to Richard Mortimer from comment #17)

Created attachment 95731 [details]
Updated trigger removal patch for MySQL databases

I forgot to note that the current patch still does not handle the deferred copy
of the icon value in the Albums table.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

1 - Each of the image, thumbnails and face recognition databases all have a table called Settings. >I can see this causing trouble with all three merged into a single database.

Which trouble exactly ? I already noticed some problem and i fixed this with
face recognition DB integration.

At a very minimum the migration needs to take care to track this and migrating back to separate >databases would be even harder. Is it going to be feasible to rename the thumbnails and face >recognition tables to something like SettingsThumb and SettingsFace or would that cause too >much trouble with SQLite support?

I thinking about to do the same. I think it's safe if Settings table differ
than SQlite for MySql.

2/ => ok.

3/ => the logic is good. turning on the bool value will stop copy processing.
But in fact an new method need to be add in this class to be able to change
this private bool member from outside (as for ex, when Cancel button is
pressed).

Granularity of bool check to cancel copy must be decreased. typically checking
bool outside the loop will prevent to break partial database copy in the middle
section of data.

In all case a garbage collector must be implemented to cleanup previous copy
while migration. Perhaps to cleanup target database can be enough.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #21 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #20)

Post by via KDE Bugzilla

1 - Each of the image, thumbnails and face recognition databases all have a table called Settings. >I can see this causing trouble with all three merged into a single database.

Which trouble exactly ? I already noticed some problem and i fixed this with
face recognition DB integration.

Sorry. That was a poor description from me. I was not referring to a specific
problem. I meant that it is likely to cause confusion/problems in the future.

Post by via KDE Bugzilla
3/ => the logic is good. turning on the bool value will stop copy
processing. But in fact an new method need to be add in this class to be
able to change this private bool member from outside (as for ex, when Cancel
button is pressed).

As I said the current code code is broken in at least 2 places. They are easy
to fix but I just wanted to record it before I forgot.

The specific issues I can see are:

If set during the "Delete all tables" loop the loop will never terminate
(m_isStopProcessing || i>=0 will always be true) and will result in a crash
once "i" becomes negative and underflows the tables array.

Similarly if set during the copying the last iteration of the copy tables loop
then "i" will become greater than the length of tables and will result in a
crash during emit stepStarted because it tries to access tables[i].

Post by via KDE Bugzilla
Granularity of bool check to cancel copy must be decreased. typically
checking bool outside the loop will prevent to break partial database copy
in the middle section of data.

agreed.

Post by via KDE Bugzilla
In all case a garbage collector must be implemented to cleanup previous copy
while migration. Perhaps to cleanup target database can be enough.

agreed. The user must expect that the target database has been changed and a
cleanup should be enough.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #22 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95746
--> https://bugs.kde.org/attachment.cgi?id=95746&action=edit
Defer migration of Albums.icon until after the Images table has been migrated

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #23 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95747
--> https://bugs.kde.org/attachment.cgi?id=95747&action=edit
Fix array under/overflow when m_isStopProcessing is set to true

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #24 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95755
--> https://bugs.kde.org/attachment.cgi?id=95755&action=edit
Move Thumbnail database Settings to ThumbSettings for MySQL

This allows the thumbnail and main image databases to be contained within the
same MySQL database.

Also ensure that thumbnail MySQL tables use the InnoDB storage engine and not
the older MyISAM engine.

I had to change the longtext fields to varchar(767) to stop MySQL complaining
on upgrade. But also note that those same fields have a 255 character long
unique index so in practice anything over 255 characters is likely to cause
problems.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95747|0 |1
is obsolete| |

--- Comment #25 from ***@gmail.com ---
Comment on attachment 95747
--> https://bugs.kde.org/attachment.cgi?id=95747
Fix array under/overflow when m_isStopProcessing is set to true

From 61762e8ed5083c791d553ddc43f278c7e08b54eb Mon Sep 17 00:00:00 2001
Date: Wed, 25 Nov 2015 21:10:23 +0000
Subject: [PATCH 3/3] Ensure that m_isStopProcessing does not cause array
overshoots
---
libs/database/coredb/coredbcopymanager.cpp | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libs/database/coredb/coredbcopymanager.cpp b/libs/database/coredb/coredbcopymanager.cpp
index e2f3115..e3cbf09 100644
--- a/libs/database/coredb/coredbcopymanager.cpp
+++ b/libs/database/coredb/coredbcopymanager.cpp
@@ -126,7 +126,8 @@ void CoreDbCopyManager::copyDatabases(const DbEngineParameters& fromDBParameters
for (int i=(tablesSize - 1); m_isStopProcessing || i >= 0; --i)
{
- if (toDBbackend.execDirectSql(QString::fromUtf8("DROP TABLE IF EXISTS %1;").arg(tables[i])) != BdEngineBackend::NoErrors)
+ if ( m_isStopProcessing ||
+ toDBbackend.execDirectSql(QString::fromUtf8("DROP TABLE IF EXISTS %1;").arg(tables[i])) != BdEngineBackend::NoErrors)
{
emit finished(CoreDbCopyManager::failed, i18n("Error while scrubbing the target database."));
fromDBbackend.close();
@@ -135,7 +136,8 @@ void CoreDbCopyManager::copyDatabases(const DbEngineParameters& fromDBParameters
}
}
- if (toDBbackend.execDirectSql(QString::fromUtf8("DROP TABLE IF EXISTS Settings;")) != BdEngineBackend::NoErrors)
+ if ( m_isStopProcessing ||
+ toDBbackend.execDirectSql(QString::fromUtf8("DROP TABLE IF EXISTS Settings;")) != BdEngineBackend::NoErrors)
{
emit finished(CoreDbCopyManager::failed, i18n("Error while scrubbing the target database."));
fromDBbackend.close();
@@ -162,7 +164,9 @@ void CoreDbCopyManager::copyDatabases(const DbEngineParameters& fromDBParameters
for (int i=0; m_isStopProcessing || i < tablesSize; ++i)
{
- emit stepStarted(i18n(QString::fromUtf8("Copy %1...").arg(tables[i]).toLatin1().constData()));
+ if (i < tablesSize) {
+ emit stepStarted(i18n(QString::fromUtf8("Copy %1...").arg(tables[i]).toLatin1().constData()));
+ }
// Now perform the copy action
--
2.5.0

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #26 from ***@gmail.com ---
Git commit e380a053a5273130e65b2d13976261cd9ca1a90a by Gilles Caulier.
Committed on 26/11/2015 at 21:50.
Pushed by cgilles into branch 'master'.

apply patch #95747 to pezvznt array overshoots in loop when cancel is applied.

M +13 -8 libs/database/coredb/coredbcopymanager.cpp

http://commits.kde.org/digikam/e380a053a5273130e65b2d13976261cd9ca1a90a

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #27 from ***@gmail.com ---
Richard,

I start to test step by step all pending patches.

For patch : Remove trigger dependency for MySQL.

==> the patch do not touch the dbsettingswidget.cpp code where SQL requirements
is given to previously create the databases in remote Mysql server by the
system admin.
==> This give a lots of error on the console when i use Mysql internal server
(i don't yet tested Mysql remote server yet). Note that Mysql Internal server
use "digikam" database name for all database (Core, Thumbs, Face). Look errors
below :

digikam.dbengine: Failure executing query:
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 1))
digikam.dbengine: Failure executing query:
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 2))
digikam.dbengine: Failure executing query:
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 3))
digikam.dbengine: Failure executing query:
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 4))
digikam.dbengine: Failure executing query:
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 5))
...

and more...

For patch : Defer migration of Albums.icon until after the Images table has been migrated

==> sound like it need the previous patch to be applied. the patch is rejected.
Did you confirm ?

For patch : Move Thumbnail database Settings to ThumbSettings for MySQL

==> We need the same solution for Face database which also create a Settings
table.
==> sound like it need the previous patch to be applied. the patch is rejected.
Did you confirm ?
==> After renaming Thumb and Face DB Settings tables, this will fix bug #331628
? At least, as you specify InnoDB database engine, TokuDB engine will be not
possible to use instead. Right ?

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #28 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #27)

Post by via KDE Bugzilla
Richard,
I start to test step by step all pending patches.

For patch : Remove trigger dependency for MySQL.

==> the patch do not touch the dbsettingswidget.cpp code where SQL
requirements is given to previously create the databases in remote Mysql
server by the system admin.

OK. I haven't looked at internal MySQL yet. I've got quite a few MySQL database
servers hanging around so it is easier for me to test and debug there in the
early stages.

Post by via KDE Bugzilla
==> This give a lots of error on the console when i use Mysql internal
server (i don't yet tested Mysql remote server yet). Note that Mysql
Internal server use "digikam" database name for all database (Core, Thumbs,

Did those errors cause the migration to fail? Or did it complete with just the
complaints on the console?

Post by via KDE Bugzilla
"UPDATE Tags SET icon=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update
a child row: a foreign key constraint fails (`digikam`.`Tags`, CONSTRAINT
`Tags_Images` FOREIGN KEY (`icon`) REFERENCES `Images` (`id`) ON DELETE SET
NULL ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(qlonglong, 0), QVariant(int, 1))

Those errors look like there are some Tags that have icons setup to point at
non-existant images. The examples you gave all have the first bound value as
zero which is typically a non-existant icon (instead of null). I suspect
most/all are set to zero instead of null. We should be able to filter out those
definitely bad ones.

What are your thoughts on filtering out data that is broken because the Album,
Image, Tag it refers to does not exist in the database? I tend to think that
the data was not accessible to users in the original database so it should be
safe to not transfer it.

Post by via KDE Bugzilla

For patch : Defer migration of Albums.icon until after the Images table has been migrated

==> sound like it need the previous patch to be applied. the patch is
rejected. Did you confirm ?

Ah sorry. I rebased my patches based on what you had already committed to the
public git and squashed updates on a single topic into a single patch. I
thought it best to make the patch in bugzilla be clear about that whole changes
for a particular reason.

Post by via KDE Bugzilla

For patch : Move Thumbnail database Settings to ThumbSettings for MySQL

==> We need the same solution for Face database which also create a Settings
table.

Yes. I did not get around to that yet. Am I correct in that we can assume no
need to provide an upgrade here because there are no existing MySQL users with
face settings yet.

Post by via KDE Bugzilla
==> sound like it need the previous patch to be applied. the patch is
rejected. Did you confirm ?

Yes. It does require the previous patch.

Post by via KDE Bugzilla
==> After renaming Thumb and Face DB Settings tables, this will fix bug
#331628 ? At least, as you specify InnoDB database engine, TokuDB engine
will be not possible to use instead. Right ?

Hmmm. A difficult problem. Really we just want to NOT use MyISAM because it
does not support referential integrity.

TokuDB should work and it would be possible to manually change all the tables
after upgrade.
If automatic support for a particular backend was required then I think it
would have to be specified on the setup/migration dialog and then manually
specified during the migration.

Richard

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Did those errors cause the migration to fail? Or did it complete with just the complaints on the console?

==> No migration here. As now first run assistant support Mysql as well, we can
satrt to init a DB with Mysql as well. So, as schema is changed a lots, i
always start tests with a fresh DB.

TokuDB should work and it would be possible to manually change all the tables after upgrade.
If automatic support for a particular backend was required then I think it would have to be >specified on the setup/migration dialog and then manually specified during the migration.

==> in fact i don't care about TokuDB engine. If we specify in table creation
that Immno engine must be used as well, because we have tested with it only,
it's fine for me.

The question is more to kill this bug with right comment, as TokuDB engine is
not supported and digiKam use Inmmo engine. The goal is not to introduce
complexity, it's already enough complex.

As Mysql is able to support tables with different engines at the same time (fix
me if i'm wrong), digiKam must be usable as well. Right ?

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #30 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #29)

Post by via KDE Bugzilla

Did those errors cause the migration to fail? Or did it complete with just the complaints on the console?

==> No migration here. As now first run assistant support Mysql as well, we
can satrt to init a DB with Mysql as well. So, as schema is changed a lots,
i always start tests with a fresh DB.

OK. I see the error now and what causes it. The AlbumDB::addTag routine in
albumdb.cpp gets passed an iconID value of zero. That presumably means "no
icon" but with referential integrity enabled the update fails because there is
no image zero in the database.

An easy solution would be to filter out any iconID = 0 values and set the icon
to null in those cases. It is not 100% safe because the database could decide
to use imageID = 0 for a real image but it would work in most cases.

I can see similar usages in setTagIcon plus the equivalent functions for Album
icons.

Post by via KDE Bugzilla

TokuDB should work and it would be possible to manually change all the tables after upgrade.
If automatic support for a particular backend was required then I think it would have to be >specified on the setup/migration dialog and then manually specified during the migration.

==> in fact i don't care about TokuDB engine. If we specify in table
creation that Immno engine must be used as well, because we have tested with
it only, it's fine for me.
The question is more to kill this bug with right comment, as TokuDB engine
is not supported and digiKam use Inmmo engine. The goal is not to introduce
complexity, it's already enough complex.

I think the correct answer is that we test and use InnoDB. There is nothing to
stop someone changing the engine to TokuDB and it should work (no guarantees)
but the engine may get changed back to InnoDB if/when digikam is upgraded.

Post by via KDE Bugzilla
As Mysql is able to support tables with different engines at the same time
(fix me if i'm wrong), digiKam must be usable as well. Right ?

Yes.


I have produced a patch to use a "FaceSettings" table for faces. Would you like
me to produce clean patches against master to change both Face and Thumbnail
databases to the new settings. I think those two are less likely to break than
the main database code because they do not use null columns in quite the same
way as the album/tags icons tables.

Richard

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #31 from ***@gmail.com ---
Richard,

With your Core DB schema update, this bug wil be fixed ?

https://bugs.kde.org/show_bug.cgi?id=286492

Gilles

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #32 from ***@gmail.com ---
Richard,

Same question than comment #31 abou this file :

https://bugs.kde.org/show_bug.cgi?id=350574

Gilles

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #33 from ***@gmail.com ---
Richard,

What's about to use Foreign Keys in DB schema ?

https://bugs.kde.org/show_bug.cgi?id=237878

It's safe since Mysql and MariaDb support it ?

Gilles

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #34 from ***@gmail.com ---
Richard,

I think file

https://bugs.kde.org/show_bug.cgi?id=281838

... is fixed since a while. Can you confirm ?

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Richard Mortimer <richm+***@oldelvet.org.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95755|0 |1
is obsolete| |

--- Comment #35 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95823
--> https://bugs.kde.org/attachment.cgi?id=95823&action=edit
Standalone MySQL Thumbnail settings to ThumbSettings table

Move Thumbnail database Settings to ThumbSettings for MySQL

This allows the thumbnail and main image databases to be contained within the
same database.

Also ensure that thumbnail MySQL tables use the InnoDB storage engine and not
the older MyISAM engine.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #36 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95824
--> https://bugs.kde.org/attachment.cgi?id=95824&action=edit
Standalone MySQL face recognition settings to FaceSettings table

Update face recognition MySQL support.

Use FaceSettings table to allow shared database in MySQL.
Use InnoDB engine to ensure that foreign key relationships are enforced.
Use foreign key between Identities and IdentityAttributes.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Richard Mortimer <richm+***@oldelvet.org.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95823|0 |1
is obsolete| |

--- Comment #37 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95825
--> https://bugs.kde.org/attachment.cgi?id=95825&action=edit
Standalone MySQL Thumbnail settings to ThumbSettings table

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #38 from ***@gmail.com ---
Thanks Richard,

I will review your patches tomorow or wenesday...

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #39 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Gilles,

1) I did not yet get chance to rebase the other two patches to separate those
from the face and thumbnail patches. I will do so within the next day.

2) I started to look at the Tags and Albums icon code. The entire code seems to
assume that an icon value of zero means no icon. This is not good for
referential integrity. Ideally all usages of qlonglong for icon value should be
replaced with something like QVariant that allows a null value too. I think
this is too big a change at the moment so my suggestion is that the database
storage code is modified to hardcode 0 as being equivalent to null in the
database. I did a quick test on both MySQL and SQLite and both work fine with
that assumption because when reading from the database a null icon is turned
into a zero when reading a longlong from QVariant that is null. Doing a full
QVariant conversion for icon fields might be something that could be included
in a GSoC project for PostgreSQL support.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #40 from ***@gmail.com ---
Git commit 04baf90347ae3365378333f6ae4f84c1dc39c73d by Gilles Caulier.
Committed on 01/12/2015 at 21:31.
Pushed by cgilles into branch 'master'.

apply patch #95824 to :
Use FaceSettings table to allow shared database in MySQL.
Use InnoDB engine to ensure that foreign key relationships are enforced.
Use foreign key between Identities and IdentityAttributes.

M +31 -16 data/database/dbconfig.xml.cmake.in
M +9 -6 libs/facesengine/facedb/facedb.cpp
M +1 -1 libs/facesengine/facedb/facedb.h

http://commits.kde.org/digikam/04baf90347ae3365378333f6ae4f84c1dc39c73d

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95824|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #41 from ***@gmail.com ---
Git commit 9b543e6a84a15af6d29f70c39a4964992f42e7cf by Gilles Caulier.
Committed on 01/12/2015 at 21:34.
Pushed by cgilles into branch 'master'.

Apply patch #95825 to :
Allows the thumbnail and main image databases to be contained within the same
database.
Also ensure that thumbnail MySQL tables use the InnoDB storage engine and not
the older MyISAM engine.

M +66 -10 data/database/dbconfig.xml.cmake.in
M +27 -4 libs/database/thumbsdb/thumbsdb.cpp
M +1 -0 libs/database/thumbsdb/thumbsdb.h
M +23 -1 libs/database/thumbsdb/thumbsdbchemaupdater.cpp
M +1 -0 libs/database/thumbsdb/thumbsdbchemaupdater.h

http://commits.kde.org/digikam/9b543e6a84a15af6d29f70c39a4964992f42e7cf

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95825|0 |1
is obsolete| |

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #42 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95860
--> https://bugs.kde.org/attachment.cgi?id=95860&action=edit
Store empty icon (image 0) as a NULL value

Tested on both SQLite and MySQL databases. Album and tag icons can be set and
cleared.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

Richard Mortimer <richm+***@oldelvet.org.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95731|0 |1
is obsolete| |
Attachment #95746|0 |1
is obsolete| |

--- Comment #43 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95861
--> https://bugs.kde.org/attachment.cgi?id=95861&action=edit
Remove coredb trigger dependency for MySQL.

This is still a work in progress.

Known issues:

1 - Tags broken on MySQL.
2 - Existing "zero" value icons break database migration.
3 - No support for upgrade of existing coredb contents on MySQL.
4 - On migration from SQLite to MySQL I noticed breakages in filenames on child
albums thumbnails. Works fine on a fresh MySQL database setup. Needs more
investigation.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #44 from ***@gmail.com ---
Git commit bca001b16dd6aba437aece1bc09e9400ec300d1d by Gilles Caulier.
Committed on 02/12/2015 at 10:50.
Pushed by cgilles into branch 'master'.

Apply patch #95860 to store empty icon (image 0) as a NULL value with SQlite
and Mysql Databases

digiKam treats image zero as a "not set" icon value. This
breaks referential integrity in the database because image
zero does not exist. Explicitly convert a value of zero
into NULL when stored in the database.

Ideally digiKam should use an explicit null (in QVariant?)
placeholder for the no-icon case but currently no database
stores an image zero by default so the existing practice
works.

Note that when reading from the database the toLongLong
method of QVariant returns a zero when called against a
null value so this means that no changes are required
when reading null values in from the database.

M +26 -5 libs/database/coredb/coredb.cpp

http://commits.kde.org/digikam/bca001b16dd6aba437aece1bc09e9400ec300d1d

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #45 from ***@gmail.com ---
Git commit 801f835f246b2b2a38a886920056c149dd6894c0 by Gilles Caulier.
Committed on 02/12/2015 at 11:04.
Pushed by cgilles into branch 'master'.

apply patch #95861 to remove trigger dependency for MySQL.

Add full referential integrity to AlbumRoots, Albums, Images and Tags
tables entries.
Provide the equivalent behaviour to the triggers using ON DELETE and
ON UPDATE in FOREIGN KEY references.

Use dbaction to perform database specific preparation for migration.

The MySQL schema has a circular dependency and this must be removed
before any leftover contents of the database are removed prior to the
migration. No actions are required for SQLite

Referential integrity in MySQL does not allow the Albums.icon field
to be set prior to the Images table being populated. Use a fake
AlbumsExtra table to copy over the icons data.
Note we use the same procedure for both SQLite and MySQL to ensure
that migration is possible between any combination of databases.

M +63 -51 data/database/dbconfig.xml.cmake.in
M +12 -0 libs/database/coredb/coredbcopymanager.cpp

http://commits.kde.org/digikam/801f835f246b2b2a38a886920056c149dd6894c0

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95861|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95860|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #46 from ***@gmail.com ---
Richard,

Did you plan to specific ImnoDb database type with Mysql for Core database
creation action, as with Thumbs and Face databases ?

If yes, this will permit to close bug #331628 as well...

Also, in Mysql, The Core database settings table is named "Settings", where for
Thumbs database is now named ThumbSettings and for Face database FaceSettings.
Why not to rename Settings table as CoreSettings to be homogeneous everywhere ?

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #47 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #46)

Post by via KDE Bugzilla
Richard,
Did you plan to specific ImnoDb database type with Mysql for Core database
creation action, as with Thumbs and Face databases ?

Yes. That seems to have got lost somewhere in all the rebasing.

Post by via KDE Bugzilla
If yes, this will permit to close bug #331628 as well...
Also, in Mysql, The Core database settings table is named "Settings", where
for Thumbs database is now named ThumbSettings and for Face database
FaceSettings. Why not to rename Settings table as CoreSettings to be
homogeneous everywhere ?

Will look to implement that when the coredb MySQL schema upgrade scripts are
written.

Richard

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #48 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95865
--> https://bugs.kde.org/attachment.cgi?id=95865&action=edit
MySQL database permissions/setup guidance streamlining.

Make it clear that the password is specific to the account/hostname combination
and not the database.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #49 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95866
--> https://bugs.kde.org/attachment.cgi?id=95866&action=edit
Specify InnoDB engine on setup for coredb tables.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #50 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95867
--> https://bugs.kde.org/attachment.cgi?id=95867&action=edit
Do not migrate "zero" icon values for Albums and Tags

MySQL referential integrity does not allow to refer to a non-existant
placeholder image. Use a null value instead.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #51 from ***@gmail.com ---
Git commit 5c1416f17b230a598b8938ee3a18bb31b65303d0 by Gilles Caulier.
Committed on 02/12/2015 at 21:21.
Pushed by cgilles into branch 'master'.

Apply patch #95865 to rewrite MySQL database creation and setup guidance.
The MySQL password is tied to the account not the database.

M +10 -8 libs/database/utils/dbsettingswidget.cpp

http://commits.kde.org/digikam/5c1416f17b230a598b8938ee3a18bb31b65303d0

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95865|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #52 from ***@gmail.com ---
Git commit 1f1855693efd9b01a7149853eb59596b1831b5f4 by Gilles Caulier.
Committed on 02/12/2015 at 21:23.
Pushed by cgilles into branch 'master'.

Apply patch #95867 to filter out existing zero (no icon set) icon values during
migration. This is required to enforce referential integrity during migration
to a MySQL database.

M +4 -4 data/database/dbconfig.xml.cmake.in

http://commits.kde.org/digikam/1f1855693efd9b01a7149853eb59596b1831b5f4

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95867|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #53 from ***@gmail.com ---
Git commit 3eefa0b7493ee7abb72d6532e618a3769e181e2f by Gilles Caulier.
Committed on 02/12/2015 at 21:28.
Pushed by cgilles into branch 'master'.

Apply patch #95866 to force use of InnoDB engine with all Mysql database table.
Related: bug 281838, bug 331628
FIXED-IN: 5.0.0

M +3 -1 NEWS
M +40 -21 data/database/dbconfig.xml.cmake.in

http://commits.kde.org/digikam/3eefa0b7493ee7abb72d6532e618a3769e181e2f

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #95866|0 |1
is obsolete| |

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #54 from ***@gmail.com ---
Richard,

The Mysql Database schema must take a care at least about these bugs :

1/ optimisations with foreign keys . I see that you start to do it with FaceDb.
I don't know if you plan for other Db.

https://bugs.kde.org/show_bug.cgi?id=237878

2/ Migration dysfunctions : i don't know if they still valid or not.

https://bugs.kde.org/show_bug.cgi?id=286492
https://bugs.kde.org/show_bug.cgi?id=350574
https://bugs.kde.org/show_bug.cgi?id=316690
https://bugs.kde.org/show_bug.cgi?id=325655

3/ Mysql and case-insensitive file path :

https://bugs.kde.org/show_bug.cgi?id=268204
https://bugs.kde.org/show_bug.cgi?id=355893

4/ Items filter which doesn't work for video type-mime :

https://bugs.kde.org/show_bug.cgi?id=327957

They are other entries to check, but we will take a look later...

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #55 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 95928
--> https://bugs.kde.org/attachment.cgi?id=95928&action=edit
Experiment with TagsTree tables.

Build a TagsTree2 file that mimics the existing behaviour
of the MySQL Tags table and use that for all tags tree queries.
The advantage of a separate table is that it can be rebuilt
without destroying or changing main Tags table.

This is experimental and it would be good if people can
exercise the relevant tags behaviour against their database
to ensure that a wide variety of queries have been tested.

If all goes well the intention would be to rename all uses
of TagsTree2 to TagsTree prior to final committal.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #56 from ***@gmail.com ---
Richard,

How to test quickly your new patch #95928 exactly ?

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #57 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #56)

Post by via KDE Bugzilla
How to test quickly your new patch #95928 exactly ?

General testing of the tags functionality with a MySQL database. I'm not 100%
sure how to test it all but a lot if it is in the searching by tag hierarchy
and viewing specific tags. I did test the queries manually but could not
excercise the the C++ code to ensure that the relevant SQL generators were
running correctly.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #58 from ***@gmail.com ---
Richard

To process C++ test with database, we can use QTest API.

http://doc.qt.io/qt-5/qtest-overview.html

Currently, we have a core/tests/ code to process this kind of check over a
small SQlite database and also MySQL. It located in albummodel and database
sub-dirs.

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/show/tests/

This code will generate a small CLI application which will process usual tests
with database schema content. The code init a DK core instance, create a
database with few image collection from data sub folder and try to run all
basic operations in database. The QTest API permit to check the results step by
step. And end we can have a resume of tests processed with success or not.

This kind of tests application is started automatically by the KDE Jenkins
server, after a complete core compilation, started after each commit.

https://build.kde.org/job/digikam%20master%20kf5-qt5/

To have all test code compiled in digiKam, use the cmake flag
-DBUILD_TESTING=ON while configuration of digiKam repository. This will enable
tests sub-dir compilation. the tests CLI tools will be compiled in build
directory and can be started as well from a console :

[***@localhost database]$ ./databasefieldstest
********* Start testing of DatabaseFieldsTest *********
Config: Using QtTest library 5.5.1, Qt 5.5.1 (x86_64-little_endian-lp64 shared
(dynamic) release build; by GCC 5.2.1 20151020)
PASS : DatabaseFieldsTest::initTestCase()
PASS : DatabaseFieldsTest::testMinSizeType()
PASS : DatabaseFieldsTest::testIterators()
PASS : DatabaseFieldsTest::testMetaInfo()
PASS : DatabaseFieldsTest::testIteratorsSetOnly()
PASS : DatabaseFieldsTest::testSet()
PASS : DatabaseFieldsTest::testSetHashAddSets()
PASS : DatabaseFieldsTest::testHashRemoveAll()
PASS : DatabaseFieldsTest::cleanupTestCase()
Totals: 9 passed, 0 failed, 0 skipped, 0 blacklisted
********* Finished testing of DatabaseFieldsTest *********

[***@localhost database]$ pwd
/home/gilles/Devel/5.x/build/core/tests/database
[***@localhost database]$

There are 3 tools :

- testdatabase : it reproduce the basis database core init and shutdown. It
used with valgrind to check if core engine has memory leak. You can forget this
one for the moment.

- databasefieldstest : this one is check if the field is DB are processed with
core engine API properly. This one do not play with tags tree.

- albummodeltest : create another small database (SQlite only for the moment)
to check DB album properties in all conditions.

So, if you need a QTest code to check tags tree in DB, i recommend to create a
new small CLI tool based on 2 last one previously described. It's not very well
complicated. init and populate a small DB (Sqlite/Mysql internal) with a small
collection of image including tags in metadata. This will load automatically
the tags tree in DB with DB scan code. After that you can try to change tags
tree as you want in code and use QTest to check if the changes results is right
or not.

Gilles

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #59 from ***@gmail.com ---
Richard, i tested your patch #95928, and i can see a lots of dupplicates tags
created with the same name when i create a new database (mysql internal).

My collection of image :

digikam version 5.0.0-beta3
Images:
JP2: 10
JPG: 1549
PGF: 7
PNG: 141
PPM: 2
RAW-ARW: 13
RAW-CR2: 45
RAW-CRW: 5
RAW-DCR: 2
RAW-DNG: 18
RAW-MRW: 18
RAW-NEF: 19
RAW-ORF: 4
RAW-PEF: 7
RAW-RAF: 10
RAW-RAW: 1
RAW-RWL: 1
RAW-X3F: 2
TIFF: 28
total: 1882
:
Videos:
MOV: 6
total: 6
:
Total Items: 1888
Albums: 164
Tags: 90
:
Database backend: QMYSQL
Database internal server: Yes
Database internal server Path: /mnt/data

While importing collection in DB, i can see also these warning, about
versionning metadata imported from XMP :

digikam.dimg: "/mnt/data/TEST_IMAGES/splashs/showfoto-splash.png" : PNG file
identified
digikam.database: Adding new item
"/mnt/data/TEST_IMAGES/splashs/showfoto-splash.png"
digikam.metaengine: DateTime => Exif.Photo.DateTimeOriginal =>
QDateTime(2010-12-29 00:00:00.000 CET Qt::TimeSpec(LocalTime))
digikam.metaengine: DateTime => Exif.Photo.DateTimeOriginal =>
QDateTime(2010-12-29 00:00:00.000 CET Qt::TimeSpec(LocalTime))
digikam.metaengine: "/mnt/data/TEST_IMAGES/splashs/showfoto-splash.png" ==>
Read Iptc Keywords: ()
digikam.database: Pick Label found : 0
digikam.database: Cannot find Pick Label Tag for : 0
digikam.database: Color Label found : 0
digikam.database: Cannot find Color Label Tag for : 0
digikam.metaengine: Loading image history ""
digikam.database: Scanning took 3 ms
digikam.database: Finishing took 1 ms
digikam.database: Folder does not exist or is not readable:
"/mnt/data/lost+found"
digikam.database: Attempt to create tag properties for tag id 0
digikam.database: items to tag (2, 178, 184, 1199, 1383, 1450, 1543, 1563,
1567, 1568, 1806, 1814, 1837, 1838, 1843, 1844, 1846, 1849, 1850, 1854, 1858,
1859, 1860, 1861, 1864, 1865, 1869)
digikam.database: Broken history: Same file referred by different entries.
Refusing to add a loop.
digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 2 UUID: e8ff0e... }"

digikam.database: Graph with 2 vertices:
"{ Ids: () UUID: a7150e... } -> { Id: 178 UUID: a00753... }"

digikam.database: Image 178 type QFlags(0x8)
digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 184 }"

digikam.database: Graph with 2 vertices:
"{ Ids: () UUID: fcac8f... } -> { Id: 1199 UUID: c9fecb... }"

digikam.database: Image 1199 type QFlags(0x8)
digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1383 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1450 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1543 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1563 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1567 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1568 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1806 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1814 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1837 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1838 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1843 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1844 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1846 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1849 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1850 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1854 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1858 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1859 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1860 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1861 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1864 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1865 }"

digikam.database: Graph with 1 vertices:
"Unconnected: { Id: 1869 }"

digikam.database: Complete scan took: 31320 msecs.

Gilles

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #60 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Gilles,

Many thanks for the information and the testing feedback. I have not had much
time to look into this yet.

A quick look at the schema does indicate that there is a missing UNIQUE clause
in the Tags table definition for MySQL. I do not think that is the full
solution but it will certainly stop duplicate entries reaching the database.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #61 from ***@gmail.com ---
Richard,

I started to write a test cases implementation for Mysql and tree-view. I need
to fix some point of Mysql database settings, especially with internal database
server that i will use for that.

Code is here but not completed and compiled for the moment :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/tests/database/databasetagstest.cpp

I will continue this week end and during this Christmas holidays...

Gilles Caulier

[Felix Leif Keppmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #62 from Felix Leif Keppmann <***@keppmann.de> ---
Hello,


currently I face the issue of not being able to completely delete directories.
Directories are deleted on disk, but not deleted from the database, i.e.,
obsolete ghost directories remain in Digikam's album tree.

Seems to be related to failing MySQL queries:

"UPDATE Albums SET albumRoot=0, relativePath=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update a
child row: a foreign key constraint fails (`digikam_core`.`Albums`, CONSTRAINT
`Albums_AlbumRoots` FOREIGN KEY (`albumRoot`) REFERENCES `AlbumRoots` (`id`) ON
DELETE CASCADE ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(QString, "1-/test"), QVariant(int, 2))

Log:
https://bpaste.net/show/120359f6c1f2

Version:
5.0.0-beta3 <-> 5.0.0-beta4
Commit c75e88ddb078cc2aef3ea3427b934fe18fda906f
Sat, 20 Feb 2016 08:35:35 +0000 (09:34 +0059)

Setup / Reproduce:
1) Digikam started with three empty MySQL databases, i.e., core, faces, and
thumbnails
2) Add test collection
3) Add test album / directory to collection
4) Delete test album -> failure, i.e., album stays in Digikam album tree,
deleted on disk, MySQL error in log
5) Delete test (ghost) album again -> no change

Maybe this helps to fix it.
As workaround, recreating a ghost album as directory manually at disk makes it
reusable in Digikam.
I run Digikam with MySQL databases and would be glad to help testing.


Regards
Felix

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

--- Comment #63 from ***@gmail.com ---
Hello Richard.

I'm Swati, currently working on digiKam MySQL database support project. I read
this open bug for MySQL Schema improvements and thought of a plausible solution
to handle the tags (rename, move or delete)

An image can have many tags and a tag can belong to multiple image. So to
handle this many-many relationship between the two, we could take use of the
following tables:

Table: Image
Columns: Image_ID, Image_Title

Table: Tag
Columns: Tag_ID, Tag_Title

Table: Image_Tag
Columns: Tag_ID, Image_ID

Setting the Image_ID and Tag_ID as foreign keys would help in linking the
tables.

Maybe this will help.

Regards
Swati

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #64 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Apologies. Life has caught up with me a little these past months. Things are
still busy but here are a few thoughts...

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #65 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to Felix Leif Keppmann from comment #62)

Post by Felix Leif Keppmann via KDE Bugzilla
currently I face the issue of not being able to completely delete
directories. Directories are deleted on disk, but not deleted from the
database, i.e., obsolete ghost directories remain in Digikam's album tree.
"UPDATE Albums SET albumRoot=0, relativePath=? WHERE id=?;"
Error messages: "QMYSQL3: Unable to execute statement" "Cannot add or update
a child row: a foreign key constraint fails (`digikam_core`.`Albums`,
CONSTRAINT `Albums_AlbumRoots` FOREIGN KEY (`albumRoot`) REFERENCES
`AlbumRoots` (`id`) ON DELETE CASCADE ON UPDATE CASCADE)" 1452 2
Bound values: (QVariant(QString, "1-/test"), QVariant(int, 2))

I think the issue here is that the database is trying to use albumRoot=0 to
mark the album as being disconnected/deleted. However with referential
integrity set this now is not allowed.

This seems to be happening in CoreDB::makeStaleAlbum(int albumID)

The lifecycle of stale albums needs considering a little more.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #66 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to swatilodha27 from comment #63)

Post by via KDE Bugzilla
Hello Richard.
I'm Swati, currently working on digiKam MySQL database support project. I
read this open bug for MySQL Schema improvements and thought of a plausible
solution to handle the tags (rename, move or delete)
An image can have many tags and a tag can belong to multiple image. So to
handle this many-many relationship between the two, we could take use of the
Table: Image
Columns: Image_ID, Image_Title
Table: Tag
Columns: Tag_ID, Tag_Title
Table: Image_Tag
Columns: Tag_ID, Image_ID
Setting the Image_ID and Tag_ID as foreign keys would help in linking the
tables.

Foreign keys have already been added here.

CONSTRAINT ImageTags_Images FOREIGN KEY (imageid) REFERENCES Images (id) ON
DELETE CASCADE ON UPDATE CASCADE,
CONSTRAINT ImageTags_Tags FOREIGN KEY (tagid)
REFERENCES Tags (id) ON DELETE CASCADE ON UPDATE CASCADE,

Without seeing a specific error I cannot tell what the problem may be with
rename, move or delete.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #67 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Apart from testing and fixing issues I am aware of the following tasks that
need completing for MySQL support.

1) Test and refine the "Experiment with TagsTree tables" patch to ensure that
it maintains the tags tree correctly.
2) Add a migration script to upgrade existing MySQL databases with the
referential integrity patches.
3) Look at image, album and tag rename, move and delete. The existing SQL
assumes too many magic "0" values and intermediate steps where referential
integrity would be broken.
4) Write unit tests for the various database operations. This should help with
the above steps.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #68 from ***@gmail.com ---
Hello Richard.

I guess then the only possible way is to reproduce every error reported by the
end user and try to generate it.

Queries in relation to the points you mentioned for completing MySQL support:
1)In the "Experiment with TagsTree tables" patch, only testing needs to be
done? To ensure that it generates expected results?

2) To add a script for migration of databases with referential integrity
patches, would it require to create a new table in the database? I'm not
actually clear on this point.

3) To accurately make image/album/tag function, referential integrity is the
best possible solution in MySQL. To check if it is broken, multiple tables in
DB need to be checked and figure out in which one a record is missing, which is
present in other tables. I guess this could be a solution?

Regards
Swati

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #69 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to swatilodha27 from comment #68)

Post by via KDE Bugzilla
Hello Richard.
I guess then the only possible way is to reproduce every error reported by
the end user and try to generate it.

Some errors may be obvious by just looking at the code. But reproducing things
is definitely a good way to start. It also means that you can test the fix!

Post by via KDE Bugzilla
1)In the "Experiment with TagsTree tables" patch, only testing needs to be
done? To ensure that it generates expected results?

Yes. I think it replicates existing behaviour.

Post by via KDE Bugzilla
2) To add a script for migration of databases with referential integrity
patches, would it require to create a new table in the database? I'm not
actually clear on this point.

No. It just requires adding "alter table" commands to convert the previous
schema to the new format. It should be pretty straightforward. But in many ways
it is easier to do that when the referential integrity setup has been tested
for new databases.

During migration there is a chance that there will be some rows that have
broken referential integrity. That said MySQL support has always been
experimental so it is probably reasonable to expect that users may need to
perform some cleanups to get an existing database to migrate to a
non-experimental version.

Post by via KDE Bugzilla
3) To accurately make image/album/tag function, referential integrity is the
best possible solution in MySQL. To check if it is broken, multiple tables
in DB need to be checked and figure out in which one a record is missing,
which is present in other tables. I guess this could be a solution?

Yes that is correct.

But note that historically Digikam has not used a strict referential integrity
solution. In a number of cases it uses zero instead of what should be null
along with other "magic" values. In order to eliminate these from the database
the actual C++ code will likely need changing to stop it relying on the magic
zero values. Much of this code is shared with SQLite so care has to be taken to
ensure that the new code works with both databases. Finding these locations
will require some analysis of the code to audit all uses of a particular field
with correction where required.

The other issue (I'm guessing here based on what I've seen so far) is that
digikam seems to perform two stage database operations in some cases. In the
deletion example it seems to update the database to mark that an object is
about to the deleted. Then it tries the deletion and if successful goes ahead
and deletes the item from the database. In the case I looked at it seemed that
referential integrity was broken between the two database operations and that
is why the operation failed in MySQL. If my assessment is correct then some
thought needs giving to how the operation can be performed in a database that
enforces referential integrity.

Regards

Richard

Post by via KDE Bugzilla
Regards
Swati

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #70 from ***@gmail.com ---
(In reply to Richard Mortimer from comment #69)

Post by Richard Mortimer via KDE Bugzilla
(In reply to swatilodha27 from comment #68)

Post by via KDE Bugzilla
Hello Richard.
I guess then the only possible way is to reproduce every error reported by
the end user and try to generate it.

Some errors may be obvious by just looking at the code. But reproducing
things is definitely a good way to start. It also means that you can test
the fix!

Post by via KDE Bugzilla
1)In the "Experiment with TagsTree tables" patch, only testing needs to be
done? To ensure that it generates expected results?

Yes. I think it replicates existing behaviour.

Post by via KDE Bugzilla
2) To add a script for migration of databases with referential integrity
patches, would it require to create a new table in the database? I'm not
actually clear on this point.

No. It just requires adding "alter table" commands to convert the previous
schema to the new format. It should be pretty straightforward. But in many
ways it is easier to do that when the referential integrity setup has been
tested for new databases.
During migration there is a chance that there will be some rows that have
broken referential integrity. That said MySQL support has always been
experimental so it is probably reasonable to expect that users may need to
perform some cleanups to get an existing database to migrate to a
non-experimental version.

Post by via KDE Bugzilla
3) To accurately make image/album/tag function, referential integrity is the
best possible solution in MySQL. To check if it is broken, multiple tables
in DB need to be checked and figure out in which one a record is missing,
which is present in other tables. I guess this could be a solution?

Yes that is correct.
But note that historically Digikam has not used a strict referential
integrity solution. In a number of cases it uses zero instead of what should
be null along with other "magic" values. In order to eliminate these from
the database the actual C++ code will likely need changing to stop it
relying on the magic zero values. Much of this code is shared with SQLite so
care has to be taken to ensure that the new code works with both databases.
Finding these locations will require some analysis of the code to audit all
uses of a particular field with correction where required.

To solve this maybe code could be reviewed in order to cross check that "0"
isn't mentioned in ELSE condition, or in place of NULL.
But I'm pretty confused on how to find locations where code needs to be
changed, so as to make it compatible with both the databases?

Post by Richard Mortimer via KDE Bugzilla
The other issue (I'm guessing here based on what I've seen so far) is that
digikam seems to perform two stage database operations in some cases. In the
deletion example it seems to update the database to mark that an object is
about to the deleted. Then it tries the deletion and if successful goes
ahead and deletes the item from the database. In the case I looked at it
seemed that referential integrity was broken between the two database
operations and that is why the operation failed in MySQL. If my assessment
is correct then some thought needs giving to how the operation can be
performed in a database that enforces referential integrity.

Referential integrity can be enforced by strictly not violating any of one of
the following:
1)No record should be allowed to add in the foreign key table, unless there
exists a corresponding record in the superior table.
2) CASCADING UPDATE to automatically update corresponding record if the primary
key record is updated.
3) CASCADING DELETE to automatically delete corresponding record if the primary
key record is deleted.

Regards
Swati

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #71 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99903
--> https://bugs.kde.org/attachment.cgi?id=99903&action=edit
Additional schema modifications from MySQL v7 to v8

Changes that need to be made for MySQL v7 to v8 transition in addition to those
already listed.

Note that I have not tested the transition from within digikam (I do not have a
current working build tree)

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #72 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99904
--> https://bugs.kde.org/attachment.cgi?id=99904&action=edit
Sample digikam v7 schema

Here is my v7 based (digikam 4.x) MySQL schema. This may be useful for testing
the upgrade from v7 to v8. I have tested the v7 upgrade commands against this
(and also against a clone of my fully populated v7 database).

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #73 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99905
--> https://bugs.kde.org/attachment.cgi?id=99905&action=edit
helper SQL command to detect invalid data in v7 MySQL databases

running these commands against a v7 database will list any data that fails the
referential integrity checks. It detects images attached to missing albums and
similar for missing tags too.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #74 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99906
--> https://bugs.kde.org/attachment.cgi?id=99906&action=edit
Helper to remove bad data prior to upgrade fromMySQL v7 to v8

Take care using this and always make a backup of your database before applying
this commands. The commands just remove any data that fails integrity checks on
the grounds that is was meaningless without being properly linked into the
image/album/tags structures.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #75 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99907
--> https://bugs.kde.org/attachment.cgi?id=99907&action=edit
Raw commands to perform a full MySQL v7 to v8 update

These can be used to test upgrades from v7 to v8 standalone from digikam.

Note that the commands do not update the internal database version numbers in
the Settings table.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #76 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
Created attachment 99908
--> https://bugs.kde.org/attachment.cgi?id=99908&action=edit
Add referential integrity constraints to a 5.0.0 v8 database

The 5.0.0 initial release database upgrade does not add referential integrity
to the database fields. The commands in this attachment perform those upgrades
and complete the proper move from v7 to v8.

Note that MySQL support in digikam has always been classed as experimental. You
are advised to use the migration facilities to cleanup any MySQL import from an
older version of the database.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #77 from ***@gmail.com ---
Richard,

I tested your patch for adding referential integrity constraints in 5.0.0 v8 DB
and it seems to work fine for me.

Thanks.

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #78 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to swatilodha27 from comment #77)

Post by via KDE Bugzilla
Richard,
I tested your patch for adding referential integrity constraints in 5.0.0 v8
DB and it seems to work fine for me.
Thanks.

Thanks for testing.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |richm+***@oldelvet.org.uk

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #79 from ***@gmail.com ---
Hi Richard

Could you please suggest more needs to be done here, after your referential
integrity patch has been implemented?

Thanks

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #80 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
The only thing that I am aware of that potentially needs addressing in MySQL is
the TagsTree support. I did note that there were some other issues addressing
that so it may already have been resolved.

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #81 from ***@gmail.com ---
Hi Richard,

Following issues have been fixed recently to ensure Tags works fine with MySQL
support:
1) Removing _DigiKam_root_tag_ from Tags tree
(https://github.com/KDE/digikam/commit/eb6318a45efe1e08f35da450633f3bcb1027fd6a)

2)Updating set fields (lft and rgt values) while moving tags in hierarchy
(https://github.com/KDE/digikam/commit/b8df6225e3cafdd720120f4bad4150fd78525e02)
And, if I'm not wrong, the lft/rgt IDs are used by search tools, and in other
places where hierarchy is concerned ?

What improvements were aimed with your Tags Tree patches? Is the current
implementation sufficient to fix all the issues that you tried with your
earlier patches ?

Thank you!

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=355831

***@pdmi.ras.ru changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@pdmi.ras.ru

[b***@kde.org]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #82 from ***@gmail.com ---
Richard,

Can we close this file since last Maik fixes for Mysql support ?

Gilles Caulier

[Richard Mortimer]

https://bugs.kde.org/show_bug.cgi?id=355831

--- Comment #83 from Richard Mortimer <richm+***@oldelvet.org.uk> ---
(In reply to caulier.gilles from comment #82)

Post by via KDE Bugzilla
Richard,
Can we close this file since last Maik fixes for Mysql support ?
Gilles Caulier

Gilles,

Yes I think we can.

Richard

[b***@kde.org]

https://bugs.kde.org/show_bug.cgi?id=355831

***@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Version Fixed In| |5.8.0
Resolution|--- |FIXED
Status|UNCONFIRMED |RESOLVED