Bug - Executing DBUpdate-to-6.pl removes 4 foreign keys

[pboguslawski]

Environment

• OS: linux
• Browser: irrelevant
• Znuny version: 6.0.43

Expected behaviour

According to #244 (comment) Znuny 6+ expects to run DBUpdate-to-6.pl on every update, even patch level.

This script should not touch already upgraded stuff.

Actual behaviour

After executing

/bin/su -c "cd /opt/otrs && /opt/otrs/scripts/DBUpdate-to-6.pl" -s /bin/bash otrs

in fresh Znuny 6.0.43 install, 4 schema items (index, constraints) are gone from db schema:

-  CONSTRAINT `FK_article_flag_article_id_id` FOREIGN KEY (`article_id`) REFERENCES `article` (`id`),
-  CONSTRAINT `FK_ticket_history_article_id_id` FOREIGN KEY (`article_id`) REFERENCES `article` (`id`),
-  KEY `FK_time_accounting_article_id_id` (`article_id`),
-  CONSTRAINT `FK_time_accounting_article_id_id` FOREIGN KEY (`article_id`) REFERENCES `article` (`id`),

How to reproduce

Install fresh Znuny 6.0.43.

Generate SQL dump of changes made by upgrade script:

otrs.Daemon.pl stop
mysqldump otrs &> pre.sql
/bin/su -c "cd /opt/otrs && /opt/otrs/scripts/DBUpdate-to-6.pl" -s /bin/bash otrs
mysqldump otrs &> post.sql
diff -Nur pre.sql post.sql &> db_changes.diff

In db_changes.diff you'll see missing stuff.

Additional information

Seems that all upgrade steps are fired on every DBUpdate-to-6.pl run but not all of them are skipped in already upgraded system. Above items are removed in scripts/DBUpdateTo6/UpgradeDatabaseStructure/ArticleTableChangesPreRename.pm (shouldn't be in already upgaded system probably) and readded in scripts/DBUpdateTo6/PostArticleTableStructureChanges.pm but this step is skipped probably in already upgraded system.

[pboguslawski]

IHMO: upgrade script on patch level updates should not introduce any heavy schema changes like adding/removing indexes, columns, etc.; such heavy, non critical, resource consuming changes should be done only on major version upgrades.

[hanneshal]

Hi @pboguslawski,

thanks for pointing this out. But the change is +-4 years old, introduced from 5 to 6 .
As far what I can tell this is an old bug
scripts/DBUpdateTo6/UpgradeDatabaseStructure/ArticleTableChangesPostRename.pm

is missing the re creation of the FKs

[pboguslawski]

just renames FKs

Upgrade script is removing mentioned items not renaming it. Constraints and one index are gone from DB after running upgrade second time after upgrade to v6.

[hanneshal]

updated my comment.

[pboguslawski]

BTW: upgrade script should not remove and recreate these items in system already upgraded to v6 but skip removing/readding and not introduce any unnecessary moves in db on patch level updates (touching huge tables costs time/resources/long downtime).

[hanneshal]

@pboguslawski It really does not help for solving this bug if you repeat this all the time. Please keep this related to the issue and not some political discussion. It may be necessary to add and remove foreign keys due to necessary changes. So please stop discussing this here in this bug. We wont change this behaviour.

Back to the cause and solution of this bug, introduced 4 years ago:
This check is the cause of the error:
scripts/DBUpdateTo6/PostArticleTableStructureChanges.pm

If a system is already on 6, it returns and does not recreate the foreign keys that were dropped previously. This is your main error.

The solution, in our opinion, is to separate the recreation of the foreign keys to a separate step after the article table check (Step 33 - Post changes on article related tables - in our test). We need to and will add this to 6.x also, to recreate the FKs.
Thanks for the report.

We don't want and cannot change the whole structure, because a migration can be aborted for some reason and we need to restart it. So we basically never know where we are coming from and what is already done.

As a side note: with every minor release drop migrations that are not needed anymore. Like migrations from previous version. Because we know that a user is coming from a minor version before. Thats one of the reasons a user can't upgrade directly from 6.0 to 6.3

[tgurr]

Has this been fixed by 1d6fa05 that has landed in the rel-6_0-dev branch so it'll probably be part of the next 6.0.x release - and will it be safe to run the migration scripts/DBUpdate-to-6.pl script in the future new release then?

[hanneshal]

Hi @tgurr
maybe I don't understand your question or the context of "safe" right, but yes. You can simply re run the script for the migration and the Foreign Keys will be added again.

Regards

[rkaldung]

Fixed with 3ee8e34 (Znuny) and 1d6fa05 (Znuny LTS 6.0)