CRM-20632: Exporting Contacts using "Export PRIMARY fields" and "Merge Household Members into their Households" produces db error

[monishdeb]

Overview

Exporting Contacts using the settings "Export PRIMARY fields" and "Merge Household Members into their Households" produces db error: "Database Error Code: Row size too large (> 8126). Changing some columns to TEXT or BLOB may help. In current row format, BLOB prefix of 0 bytes is stored inline., 1118"

Before

After

It fixes this error, also the patch is tested in 5.7 on FULL_GROUP_BY_MODE enabled.

Technical Details

Fix need a minor modification in appendAnyValueToSelect(...) to support alias when there isn't any and when select column is used in aggregate fn. On the other hand, modified the existing UT to include mergeSameHousehold = TRUE criteria.

---

• CRM-20632: Exporting Contacts using "Export PRIMARY fields" and "Merge Household Members into their Households" produces db error

[monishdeb]

ping @lcdservices

[monishdeb]

Jenkins test this please

[lcdservices]

@monishdeb I implemented this and ran tests. I no longer get the error, but the merge does not happen.

• created two test individuals and one household
• created household member relationships between them
• searched for all three contacts and triggered export
• chose primary export and option to merge household members
• resulting file had all three contacts listed. it should have only had the household record.

[lcdservices]

Ran through tests, and we're making progress.

1. exported primary fields + merge households = single household record exported (correct result)
2. exported primary fields + same address = household + combined indivs (should have only exported the household; the existence of a household overrides the same address merge; it does correctly match and merge records that have the same address though)
3. exported select fields + merge households = single household record exported (correct)
4. exported select fields + same address = household + combined indivs (same issue as 2)

So it looks like we just need to sort out the issue with the merge same address function so that if a household relationship exists, it uses that instead of constructing the merged record. The help text for that field explains the intended behavior.

[eileenmcnaughton]

If you are touching this code you should also remove the cloning per #11703 - it's a bit of a timebomb in the code because of the way it interacts with the memory leak bug in the DAO.

[monishdeb]

@lcdservices re 2nd and 4rth point, isn't this a separate issue as the current patch only fixes the fatal error related to Merge Household Members into their Households NOT Merge All Contacts with the Same Address ? The current code only refactor mergeSameHousehold() fn and it's related code. Also it doesn't bring any unintended affect to Merge All Contacts with the Same Address scenario.

@eileenmcnaughton thanks for pointing that memory leak case, will bring up the change in next PR update.

[lcdservices]

@monishdeb I assumed (perhaps incorrectly) that these changes may have impacted that functionality. If you think otherwise, let's break it out into a separate issue (we will need to address it one way or the other).

[eileenmcnaughton]

@monishdeb extractions are really good for the code base but hard to review when combined with other changes - if you do a clean extraction as a stand alone PR then I can look to merge that & this will be a lot simpler

[monishdeb]

@eileenmcnaughton I am unable to extract the code into a separate PR because of this variable $relationType which is being used in every change. Let me explain what I did in terms of LOC:

1. L572-582: Initialised variable $relationType which will contain relationship direction (i.e. 8_a_b) and temp table as its value. This temp table will store the related household record, whereas earlier a single table was used to store these data on separate set of columns appended with relationship prefix i.e. 8_a_b_contact_type, 8_a_b_display_name etc. Later leads to the current issue, reason why we are recording the related Household data in a separate table and later merged with it's member.
2. L848 : moved the chunk of codes in a separate function fetchRelationshipDetails() that stores the related 8_a_b or 7_b_a records in a separate array structure like

$row = [
id,
contact_type,
...
8_a_b => [
   id,
   ...
  ]
7_b_a => [
 ...
 ],
]

unlike earlier as $row = [..., 8_a_b_id, ...,7_b_a_id...]

3. L942-944: Is about creating and assigning temp table for respective relationship types as explained in step 1.

4. Changes in writeDetailsToTable() function is to INSERT records of related contacts in those separate temp tables here

5. Changes in mergeSameHousehold() is to UPDATE the main temp table that contain all the exported contacts, with the related records of Household contacts via INNER JOIN on relationship temp table (that contain corresponding Household records). This is the point

6. L1845 - is a point where we doing GROUP_CONCAT on select columns except civicrm_primary_id which is used in GROUP BY. This is to avoid FULL_GROUP_BY_MODE DB error on Mysql 5.7

7. L2028-2087 is no longer required as a result of change in logic.

8. Extended ability of appendAnyValueToSelect to retain the SELECT column name by appending alias

[monishdeb]

@eileenmcnaughton I have merged #11918 and rebased this PR. Please have a look.

[monishdeb]

Added UT

[monishdeb]

Jenkins test this please

[eileenmcnaughton]

@monishdeb current iteration towards getting this merged is #12290

[eileenmcnaughton]

@monishdeb I'm going to close this now & keep chipping away at rationalising the export function. The current refactor is here #12579 and #12586

I'm now about 95% sure we gain no performance benefit by ever writing the household merge fields to a table as we actually iterate through them in php and we have all the information we need to merge them in php at a time when we are already doing heavy processing.