Temporary tables should follow consistent naming convention
#tldr
- check #183 (comment 5717) for how to use the new syntax
Context
For epic:ro-db
, we aim to allow CiviCRM PHP workers to connect to read-only MySQL slaves and run searches+reports. These often require temporary tables.
By default, if you have replicated MySQL configuration, MySQL replicates TEMPORARY tables. However, there isn't much point in doing this because TEMPORARY tables are session-scoped. (If there's a fault, you can't recover access to them.) One can disable replication by setting an option like --replicate-wild-ignore-table=<wildcard>
. See also:
https://dev.mysql.com/doc/refman/5.7/en/replication-features-temptables.html
A recommended practice when using statement-based or mixed-format replication is to designate a prefix for exclusive use in naming temporary tables that you do not want replicated...
If you search the CiviCRM source tree for temp tables, you can find a variety of table name formats... such as civicrm_temp_exampledata
or civicrm_exampledata_temp_{$rand}
or X_exampledata
or just exampledata
. Fortunately, these are amenable to change: they’re easy to find with “grep”, and (by virtue of being temporary tables) they’re only referenced a couple times. However, there are a large number of them, and (after renaming a temp-table) you should r-run
the affected code to ensure it works.
Note: The benefit of the change will likely accrue on systems which use a replicated master/slave topology on MySQL v5.6+. However, we don't need to test that specific configuration. Our focus here is just on the naming.
Goal
Enforce a common naming convention for temporary tables:
-
civicrm_tmp_e_%
matches any true temp table (i.e. "temporary-during-this-connection"; CREATE TEMPORARY TABLE). The "e" stands for "ephemeral". -
civicrm_tmp_d_%
matches any quasi-temp table (i.e. "temporary-for-a-few-minutes-or-hours"; CREATE TABLE). The "d" stands for "durable". -
civicrm_tmp_%
matches any temp table, whether ephemeral or durable.
This arrangement will give the MySQL DBA some decent ways to scope their policies.
To generate a table name, one can hard-code the prefix or use a helper. Following 12311, you can use the helper CRM_Utils_SQL_TempTable
, e.g.
The general goal is update calls to createTempTableName()
(or to manually-constructed table-names) with the newer convention, and then re-test anything that have used it.
Changing the prefix on temp tables should usually be pretty safe/portable, provided that we do some basic searching/testing after each item is renamed, i.e.
- Search code-base for anything that might have used the temporary-table name.
- Run a use-case or two that involves actually creating the temporary table.
Audit list
Just to help track progress, here's a list of files with tell-tale signs that they may be using temporary (ephemeral or durable) tables. Here's the process for auditing each file:
- Search for the file for any references to the target phrase (
CREATE TEMPORARY TABLE
orCREATE TABLE
orcreateTempTableName
). - Determine if the file is creating a temporary (ephemeral or durable) table. Check if the table name matches either
civicrm_tmp_e_%
orcivicrm_tmp_d_%
.- If it doesn't really create a temp-table, we're done.
- If it creates a compliant temp-table, we're done.
- If it creates a non-compliant temp-table, then we need to patch.
- Figure out use-case(s) which hit that temp-table.
- To see if you've found a real use-case, it helps to hack the code in question and provoke an error. (eg
throw new Exception('Oooga booga')
) - (NB: we're being optimistic that if one use-case works for the table that they all will. We're expecting failures to come out in the wash.)
- To see if you've found a real use-case, it helps to hack the code in question and provoke an error. (eg
- Change the temp-table name. Search+update anything that references the same temp table. Verify the use-case still works as before.
- Submit the patch. (If it's a new PR, include the link.)
- Check the item as done. (The PR doesn't have to be merged -- we just to ensure that this item isn't re-examined.)
Files: (note some of these have been fixed without being checked off)
- Files matching
CREATE TEMPORARY TABLE
-
CRM/Activity/BAO/Activity.php
-
CRM/Campaign/BAO/Query.php
-
CRM/Campaign/Form/Task/Interview.php
-
CRM/Contact/BAO/GroupContactCache.php
-
CRM/Contact/BAO/Query.php
-
CRM/Contact/Form/Search/Custom/ContribSYBNT.php
-
CRM/Contact/Form/Search/Custom/DateAdded.php
-
CRM/Contact/Form/Search/Custom/FullText.php
-
CRM/Contact/Form/Search/Custom/Group.php
-
CRM/Contact/Form/Search/Custom/PriceSet.php
-
CRM/Contact/Form/Search/Custom/RandomSegment.php
-
CRM/Contact/Import/Form/DataSource.php
-
CRM/Contribute/BAO/Query.php
-
CRM/Dedupe/BAO/QueryBuilder/IndividualUnsupervised.php
-
CRM/Dedupe/BAO/RuleGroup.php
-
CRM/Logging/ReportSummary.php
-
CRM/Mailing/BAO/Mailing.php
-
CRM/Mailing/BAO/Recipients.php
-
CRM/Mailing/Event/BAO/Delivered.php
-
CRM/Report/Form/Activity.php
-
CRM/Report/Form/ActivitySummary.php
-
CRM/Report/Form/Contribute/Bookkeeping.php
-
CRM/Report/Form/Contribute/Detail.php
-
CRM/Report/Form/Contribute/Lybunt.php
-
CRM/Report/Form/Contribute/Repeat.php
-
CRM/Report/Form/Member/ContributionDetail.php
-
CRM/Report/Form.php
-
CRM/Upgrade/Incremental/php/FourThree.php
-
CRM/Upgrade/Incremental/php/FourTwo.php
-
CRM/Utils/SQL/TempTable.php
-
Civi/Install/Requirements.php
-
tests/phpunit/api/v3/MailingTest.php
-
install/index.php
-
- Files matching
CREATE TABLE
-
CRM/Contact/Form/Task.php
-
CRM/Contact/Import/Form/DataSource.php
-
CRM/Contact/Import/ImportJob.php
-
CRM/Core/BAO/SchemaHandler.php
-
CRM/Core/DAO.php
-
CRM/Export/BAO/Export.php
-
CRM/Import/DataSource/CSV.php
-
CRM/Logging/Schema.php
-
CRM/Upgrade/Incremental/php/FourFour.php
-
CRM/Upgrade/Incremental/php/FourSeven.php
-
CRM/Upgrade/Incremental/sql/4.2.alpha1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.2.beta1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.2.beta6.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.3.alpha1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.4.alpha1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.5.alpha1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.6.3.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.6.5.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.6.alpha1.mysql.tpl
-
CRM/Upgrade/Incremental/sql/4.7.alpha1.mysql.tpl
-
CRM/Utils/SQL/TempTable.php
-
CRM/Utils/String.php
-
Civi/Core/InstallationCanary.php
-
Civi/Install/Requirements.php
-
Civi/Test/CiviEnvBuilder.php
-
tests/phpunit/api/v3/CustomApiTest.php
-
tests/phpunit/api/v3/dataset/uf_group_contact_activity_26.xml
-
tests/phpunit/api/v3/LoggingTest.php
-
tests/phpunit/CRM/Contact/Import/Form/MapFieldTest.php
-
tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
-
tests/phpunit/CRM/Core/DAOTest.php
-
tests/phpunit/CRM/Logging/LoggingTest.php
-
tests/phpunit/CRM/Logging/SchemaTest.php
-
tests/phpunit/CRM/Utils/QueryFormatterTest.php
-
install/index.php
-
- Files matching
createTempTableName
-
CRM/Campaign/BAO/Query.php
-
CRM/Campaign/Form/Task/Interview.php
-
CRM/Contact/BAO/Query.php
-
CRM/Core/DAO.php
-
CRM/Report/Form/ActivitySummary.php
-
CRM/Report/Form/Contribute/Bookkeeping.php
-
CRM/Upgrade/Incremental/php/FourThree.php
-
CRM/Upgrade/Incremental/php/FourTwo.php
-
tests/phpunit/CRM/Core/DAOTest.php
-
Audit Mentality
The deliverables are patches -- hopefully mostly simple patches -- but this is a bit misleading. If we just wanted simple patches to some files, there are faster ways to edit the files. The main work in here is the audit -- ie when we look at a file, figure out how it's used. That's why we have to do searching+testing as part of the update.