From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 16 Mar 2021 03:22:21 +0000 (+1300)
Subject: Fix for dedupe error under some mysql configs
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=b520bbf0d96ca763c2643f96d4514ea03124b423;p=civicrm-core.git

Fix for dedupe error under some mysql configs

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
;
---

diff --git a/CRM/Dedupe/BAO/Rule.php b/CRM/Dedupe/BAO/Rule.php
index b695d18b6e..6b7f63be7b 100644
--- a/CRM/Dedupe/BAO/Rule.php
+++ b/CRM/Dedupe/BAO/Rule.php
@@ -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} <> ''";
     }
diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index 8dfc7d6295..01d833e72b 100644
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -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,
+    ];
+  }
+
 }