CRM_Core_BAO_CustomField::getChangeSerialize always returns a change

Overview

Editing certain types of custom field where multi-select in possible results in page hanging and fatal error returned by Ajax. This results in CiviCRM attempting to use the "CRM_Core_DAO::VALUE_SEPARATOR" character to concatenate MySQL fields which may be integers. At that point MySQL throws an error for (eg) [nativecode=1067 ** Invalid default value for 'myintegerfield_year_142'] because the code is trying to enter a non-integer 'serialized' value.

Problem originally identified on Stack Exchange

Although this is a bug on all platforms, I have only been able to trigger the error on a Windows server. I presume this relates somehow to encoding of the "CTRL-A" character used as a serialization separator, which is converted to a string representation (eg '\x01').

Reproduction steps

1. Create a custom date field or select field via Administer > Custom Fields.
2. Ensure that any select field does NOT have 'multi-select' enabled.
3. Ensure that the field type to be stored is not a string (ie DATETIME or INT).
4. Save.
5. Edit the field you have just created. Change anything, but leave multi-select disabled as before.
6. Click save.

Current behaviour

Page appears to hang with a spinning 'activity happening' cursor. Consulting data returned by Ajax call and logs reveals a fatal MySQL error arising from attempting to save a string into an integer field.

Extract from error log (full error attached)error.txt:

"UPDATE `civicrm_value_tax_history_7` SET `date_completed_173` = SUBSTRING_IND...", "1292 ** Incorrect datetime value: '%' for column 'date_completed_173' at row 1

Expected behaviour

Updated custom field should save without errors.

Environment information

• CiviCRM: 5.47.2, master
• PHP 7.3
• Wordpress 5.8
• MySQL 5.7.34
• Apache 2.4.48

Suggested fix/patch

The following code appears not to account for a serialization status passed in as 'null' (which is passed to the method in the $params array as a string, rather than a null value) instead of boolean 0.

Current code

 protected static function getChangeSerialize($params) {
    if (isset($params['serialize']) && !empty($params['id'])) {
        if($params['serialize'] == 'null'){
            $params['serialize'] = 0;
        }
      if ($params['serialize'] != CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $params['id'], 'serialize')) {
        return (int) $params['serialize'];
      }
    }
    return NULL;
  }

Suggested fix

 protected static function getChangeSerialize($params) {
    if (isset($params['serialize']) && !empty($params['id'])) {
        if($params['serialize'] == 'null'){
            $params['serialize'] = 0;
        }
      if ($params['serialize'] != CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $params['id'], 'serialize')) {
        return (int) $params['serialize'];
      }
    }
    return NULL;
  }

It's possible that the fix would be better handled wherever a '0' value for a civicrm_custom_field.serialize field is converted to (string)'null' rather than an INT, but the fix above resolved the problem for me.

Suggested patch