Fix for dedupe error under some mysql configs
authoreileen <emcnaughton@wikimedia.org>
Tue, 16 Mar 2021 03:22:21 +0000 (16:22 +1300)
committereileen <emcnaughton@wikimedia.org>
Tue, 16 Mar 2021 03:22:21 +0000 (16:22 +1300)
A dedupe rule with state_province_id in it will result in a fatal error with the
following mysql modes (it seems that all 3 must be in play)
IGNORE_SPACE,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO

I can replicate this locally in the unit test CRM_Dedupe_MergerTest::testGetMatchesIgnoreLocationType
with this PR https://github.com/civicrm/civicrm-core/pull/19817 applied
- we can discuss how to best add cover for stricter mysql
configs on that PR / chat. I am running
10.4.17-MariaDB-1:10.4.17+maria~focal locally

The logic of this is that we only want to prevent empty comparisons
for strings both because that is where they are meaningful
and because they can cause queries to fail if compared to a string.

Note that this PR https://github.com/civicrm/civicrm-core/pull/13538
blocked the = '' comparison for date fields (specifically birth_date)
but added an unneeded comparison in it's place - we don't have
a practice of putting zeros in date fields to denote no date is
present.

Hence I have removed the empty check for all types that are not text-like.

The following sql reliably produces the bug for me

SET GLOBAL sql_mode = 'IGNORE_SPACE,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION';
DROP TEMPORARY TABLE IF EXISTS civicrm_tmp_e_dedupe_86214b585206800bf06fc0344a2d52ff;
CREATE TEMPORARY TABLE
(
  id1 INT,
  id2 INT,
  weight INT,
  UNIQUE UI_id1_id2 (id1, id2)
) ENGINE = InnoDB
COLLATE utf8mb4_unicode_ci;

INSERT INTO civicrm_tmp_e_dedupe_86214b585206800bf06fc0344a2d52ff
  (id1, id2, weight)
SELECT t1.contact_id id1, t2.contact_id id2, 5 weight
FROM civicrm_address t1
     INNER JOIN civicrm_address t2
                ON (t1.state_province_id IS NOT NULL
                  AND t2.state_province_id IS NOT NULL
                  AND t1.state_province_id = t2.state_province_id
                  AND t1.state_province_id <> ''''
                  AND t2.state_province_id <> '''')
WHERE t1.contact_id < t2.contact_id
  AND t2.contact_id = 205
;

CRM/Dedupe/BAO/Rule.php
CRM/Utils/Type.php

index b695d18b6e95f586d5962a949a698520874510a8..6b7f63be7b84b468e87845a18bed06185a4269de 100644 (file)
@@ -65,11 +65,8 @@ class CRM_Dedupe_BAO_Rule extends CRM_Dedupe_DAO_Rule {
       "t2.{$this->rule_field} IS NOT NULL",
       "t1.{$this->rule_field} = t2.{$this->rule_field}",
     ];
-    if ($this->getFieldType($this->rule_field) === CRM_Utils_Type::T_DATE) {
-      $innerJoinClauses[] = "t1.{$this->rule_field} > '1000-01-01'";
-      $innerJoinClauses[] = "t2.{$this->rule_field} > '1000-01-01'";
-    }
-    else {
+
+    if (in_array($this->getFieldType($this->rule_field), CRM_Utils_Type::getTextTypes(), TRUE)) {
       $innerJoinClauses[] = "t1.{$this->rule_field} <> ''";
       $innerJoinClauses[] = "t2.{$this->rule_field} <> ''";
     }
index 8dfc7d6295063084a44c53bbf44cdebf0b814150..01d833e72b829a9a2035e10cd81edab9588c1e33 100644 (file)
@@ -529,4 +529,30 @@ class CRM_Utils_Type {
     return array_combine($types, $types);
   }
 
+  /**
+   * Get all the types that are text-like.
+   *
+   * The returned types would all legitimately be compared to '' by mysql
+   * in a query.
+   *
+   * e.g
+   * WHERE display_name = '' is valid
+   * WHERE id = '' is not and in some mysql configurations and queries
+   * could cause an error.
+   *
+   * @return array
+   */
+  public static function getTextTypes(): array {
+    return [
+      self::T_STRING,
+      self::T_ENUM,
+      self::T_TEXT,
+      self::T_LONGTEXT,
+      self::T_BLOB,
+      self::T_EMAIL,
+      self::T_URL,
+      self::T_MEDIUMBLOB,
+    ];
+  }
+
 }