Details
-
Type: Bug
-
Status: Open
-
Priority: Trivial
-
Resolution: Unresolved
-
Affects Version/s: 4.7.16
-
Fix Version/s: None
-
Component/s: None
-
Labels:None
-
Versioning Impact:Patch (backwards-compatible bug fixes)
-
Documentation Required?:Developer Doc
-
Funding Source:Needs Funding
-
Verified?:No
Description
In upgrades and other processes (eg disable/enable localization, see CRM-19968) I've seen repeated breakages where CiviCRM attempted some SQL and expectations were not met.
For example, DROP TABLE where table does not exist, or CREATE TABLE where it does, ALTER TABLE to add indexes etc.
These can either be results of bad process (perhaps a failed previous attempt to enable localisation, perhaps loading an older MySQL DB over a DB which already contains tables created in later version) or they can be situations CiviCRM didn't handle correctly (edge cases, don't have a handy example). I think we can handle them more robustly.
I believe there's a general move to avoid table modifications in upgrade via SQL include (CRM/Upgrade/Incremental/sql), and to prefer PHP update functions (CRM/Upgrade/Incremental/php). I think the latter approach is superior, it lets us check expectations before trying updates and better handle unexpected cases.
MySQL also has facilities to use IF EXISTS in SQL, and can do this from within MySQL version-specific comments. I think it's clearer to read if we do this in PHP by checking the MySQL version first, but want to note that the comment syntax is an available option.
A few ideas we could consider:
- Consider a style check warning to deprecate further SQL-only update functions being added to CRM/Upgrade/Incremental/php
- Ensure future SQL table changes have appropriately robust behaviour. Perhaps some wrapper functions that make schema alteration easier, and reduce usage of CRM_Core_DAO::executeQuery() to achieve same.
- Perhaps review existing alteration statements and apply similar changes to those.
Opening JIRA for discussion ...