Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1725 Remove groups, tags and notes from the returned data fo… #17444

Merged
merged 1 commit into from
Jun 3, 2020

Conversation

seamuslee001
Copy link
Contributor

…r the usage in preview data to speed up loading of the select fields screen

Overview

This aims to speed up the generation of the select fields for export screen after doing an advanced search by removing the groups, tags and notes return properties

Before

slow to load the select fields for export screen due to slow joins / select query

After

Faster to load the screen

ping @eileenmcnaughton @jitendrapurohit

@civibot
Copy link

civibot bot commented Jun 1, 2020

(Standard links)

@civibot civibot bot added the master label Jun 1, 2020
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 so the issues are

  1. is there agreement to remove these from primary fields? These don't necessarily perform poorly - e.g WMF doesn't have searchPrimaryOnly turned on & has no trouble exporting with these fields. (& dev/core#1725 Only export primary address fields #17458 addresses the primary fields)
  2. if so then the right place would be in the exportProcessor class rather than the form.

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton I'm not proposing to completely remove them from primary fields, I'm just proposing to essentially exclude them for the purposes of generating the Preview of data. The problem is that for sites such as AUG and maybe even Pete's sites having a large number of groups and or tags or notes really slows down the export process which is why AUG has actually disabled the export by primary fields option on exports.

This retains the standard field set if someone does an export of primary fields but when generating the content for the preview it doesn't generate any content for the groups or tags or notes fields if they are selected.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 Ah ok - & what about when it 's not primary fields only - should we preview them then?

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 FYI the sites @petednz had that I looked at had searchPrimaryOnly = FALSE

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton this preview of data is only shown on the select fields screen when you have selected to not use the primary fields export, The problem is that because NULL is the 2nd param to the ExportProcessor constructor it defaults to using the default return params which for a Contact Export have groups and tags etc included. So all I am doing is manually manipulating the return params for the purpose of the preview to ensure that those fields are removed

I know AUG have searchPrimaryOnly = TRUE and still ground to the knees

@jitendrapurohit
Copy link
Contributor

@seamuslee001 @eileenmcnaughton Thanks for the PR.

Have applied this on the site with search primary only = no.

With the above setting unchanged, this improves the time involved in export when the option "select fields to export" is chosen.

Export Primary fields is consuming the same time to download the file. No performance improvement was observed with this option. @seamuslee001 was it the same for you as well?

I tried to force the primary option by applying #17458 PR to the site. The query formed do have the condition of table.is_primary = 1 but it still takes too long to complete the process.

Full Query -

SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, contact_a.display_name as `display_name`, contact_a.do_not_email as `do_not_email`, contact_a.do_not_phone as `do_not_phone`, contact_a.do_not_mail as `do_not_mail`, contact_a.do_not_sms as `do_not_sms`, contact_a.do_not_trade as `do_not_trade`, contact_a.is_opt_out as `is_opt_out`, contact_a.legal_identifier as `legal_identifier`, contact_a.external_identifier as `external_identifier`, contact_a.nick_name as `nick_name`, contact_a.legal_name as `legal_name`, contact_a.image_URL as `image_URL`, contact_a.preferred_communication_method as `preferred_communication_method`, contact_a.preferred_language as `preferred_language`, contact_a.preferred_mail_format as `preferred_mail_format`, contact_a.hash as `hash`, contact_a.source as `contact_source`, contact_a.first_name as `first_name`, contact_a.middle_name as `middle_name`, contact_a.last_name as `last_name`, contact_a.prefix_id as `prefix_id`, contact_a.suffix_id as `suffix_id`, contact_a.formal_title as `formal_title`, contact_a.communication_style_id as `communication_style_id`, contact_a.email_greeting_id as email_greeting_id, contact_a.postal_greeting_id as postal_greeting_id, contact_a.addressee_id as addressee_id, contact_a.job_title as `job_title`, contact_a.gender_id as `gender_id`, contact_a.birth_date as `birth_date`, contact_a.is_deceased as `is_deceased`, contact_a.deceased_date as `deceased_date`, contact_a.household_name as `household_name`, IF ( contact_a.contact_type = 'Individual', NULL, contact_a.organization_name ) as organization_name, contact_a.sic_code as `sic_code`, contact_a.user_unique_id as `user_unique_id`, contact_a.employer_id as `current_employer_id`, contact_a.is_deleted as `contact_is_deleted`, contact_a.created_date as `created_date`, contact_a.modified_date as `modified_date`, contact_a.addressee_display as addressee_display, contact_a.addressee_custom as addressee_custom, contact_a.email_greeting_display as email_greeting_display, contact_a.email_greeting_custom as email_greeting_custom, contact_a.postal_greeting_display as postal_greeting_display, contact_a.postal_greeting_custom as postal_greeting_custom, IF ( contact_a.contact_type = 'Individual', contact_a.organization_name, NULL ) as current_employer, civicrm_address.id as address_id, civicrm_location_type.id as location_type_id, civicrm_location_type.name as `location_type`, civicrm_address.street_address as `street_address`, civicrm_address.street_number as `street_number`, civicrm_address.street_number_suffix as `street_number_suffix`, civicrm_address.street_name as `street_name`, civicrm_address.street_unit as `street_unit`, civicrm_address.supplemental_address_1 as `supplemental_address_1`, civicrm_address.supplemental_address_2 as `supplemental_address_2`, civicrm_address.supplemental_address_3 as `supplemental_address_3`, civicrm_address.city as `city`, civicrm_address.postal_code_suffix as `postal_code_suffix`, civicrm_address.postal_code as `postal_code`, civicrm_address.geo_code_1 as `geo_code_1`, civicrm_address.geo_code_2 as `geo_code_2`, civicrm_address.manual_geo_code as `manual_geo_code`, civicrm_address.name as `address_name`, civicrm_address.master_id as `master_id`, civicrm_address.county_id as county_id, civicrm_address.state_province_id as state_province_id, civicrm_address.country_id as country_id, civicrm_phone.id as phone_id, civicrm_phone.phone_type_id as `phone_type_id`, civicrm_phone.phone as `phone`, civicrm_phone.phone_ext as `phone_ext`, civicrm_email.id as email_id, civicrm_email.email as `email`, civicrm_email.on_hold as `on_hold`, civicrm_email.is_bulkmail as `is_bulkmail`, civicrm_email.signature_text as `signature_text`, civicrm_email.signature_html as `signature_html`, civicrm_im.id as im_id, civicrm_im.provider_id as `im_provider`, civicrm_im.provider_id as provider_id, civicrm_im.name as `im`, civicrm_openid.id as openid_id, civicrm_openid.openid as `openid`, civicrm_worldregion.id as worldregion_id, civicrm_worldregion.name as `world_region`, civicrm_website.id as website_id, civicrm_website.url as `url`, 
            CONCAT_WS(',',
            GROUP_CONCAT(DISTINCT IF(civicrm_group_contact.status = 'Added', civicrm_group_contact.group_id, '')),
            GROUP_CONCAT(DISTINCT civicrm_group_contact_cache.group_id)
          )
          as `groups`, GROUP_CONCAT(DISTINCT(civicrm_tag.name)) as tags, GROUP_CONCAT(DISTINCT(civicrm_note.note)) as notes, civicrm_activity.id as activity_id, civicrm_activity.activity_type_id as `activity_type_id`, civicrm_activity.status_id as `activity_status_id`  

FROM civicrm_contact contact_a   
  LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 )  
  LEFT JOIN civicrm_country ON ( civicrm_address.country_id = civicrm_country.id )  
  LEFT JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id AND civicrm_email.is_primary = 1)  
  LEFT JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id AND civicrm_phone.is_primary = 1)  
  LEFT JOIN civicrm_im ON (contact_a.id = civicrm_im.contact_id AND civicrm_im.is_primary = 1)  
  LEFT JOIN civicrm_openid ON ( civicrm_openid.contact_id = contact_a.id AND civicrm_openid.is_primary = 1 )  
  LEFT JOIN civicrm_location_type ON civicrm_address.location_type_id = civicrm_location_type.id  
  LEFT JOIN civicrm_group_contact ON contact_a.id = civicrm_group_contact.contact_id  
  LEFT JOIN civicrm_group_contact_cache ON contact_a.id = civicrm_group_contact_cache.contact_id  
  LEFT JOIN civicrm_entity_tag ON ( civicrm_entity_tag.entity_table = 'civicrm_contact' AND civicrm_entity_tag.entity_id = contact_a.id )  
  LEFT JOIN civicrm_note ON ( civicrm_note.entity_table = 'civicrm_contact' AND contact_a.id = civicrm_note.entity_id )  
  LEFT JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id  
  LEFT JOIN civicrm_activity_contact ON ( civicrm_activity_contact.contact_id = contact_a.id )  
  LEFT JOIN civicrm_activity ON ( civicrm_activity.id = civicrm_activity_contact.activity_id 
    AND civicrm_activity.is_deleted = 0 AND civicrm_activity.is_current_revision = 1 ) 
  INNER JOIN civicrm_contact ON ( civicrm_activity_contact.contact_id = civicrm_contact.id and civicrm_contact.is_deleted != 1 )    
  LEFT  JOIN civicrm_tag ON civicrm_entity_tag.tag_id = civicrm_tag.id  
  LEFT JOIN civicrm_website ON contact_a.id = civicrm_website.contact_id  
  
WHERE  ( civicrm_activity.activity_type_id IN ("110") 
  AND civicrm_activity.status_id IN ("2", "1") 
  AND  civicrm_activity_contact.record_type_id = 3 
  AND civicrm_activity.is_test = 0 )  
  AND (contact_a.is_deleted = 0) 
  AND contact_a.is_deleted != 1 
  AND  contact_a.id IN ( 51295,9565,9482,9476,9634,49694,5613,35926,694,48243,3670,2072,10042,12232,11487,51258,2573,51350,26900,1330,25935,10189,734,25972,3984,493,2785,1651,732,9632,43794,749,1879,10760,48430,26021,9425,18279,32071,37583,2786,4131,51354,3873,2229,9572,34647,15310,11247,9673,28028,19611,727,45352,51349,9400,4010,1416,1446,1453,48129,20548,20906,38530,1827,31800,18456,38762,49283,26890,3303,51211,11224,51193,51213,51373,20930,38441,9797,38174,9344,10157,43270,28243,50561,4225,48518,14762,37470,50450,5709,20957,9520,51251,11495,37362,28206,3675,12767,717,1245,1128,1709,9325,716,20993,384,15178,2600,42962,2267,9558,36265,9309,3607,43244,43756,705,1439,2521,24198,715,5685,51322,714,48954,753,17180,51387,3001,4413,51357,1232,9653,51166,18390,34777,51276,51281,51324,17103,26820,4159,50515,774,709,39728,51321,19150,9259,27804,4081,41418,44866,39673,31115,45100,45535,20170,9668,3216,11257,51003,34712,3933,51368,51181,43428,2717,2173,43020,11687,38154,31845,47306,51171,9737,48261,11148,21302,3203,3365,5807,39831,30572,43960,51134,21400,49164,11890,51345,19202,38366,50562,34355,17139,48823,18670,3983,2920,51315,4301,11673,51382,50459,10303,3232,9659,5563,51365,9141,9148,40368,48622,683,51106,685,43124,1360,33661,680,51327,51140,51317,51435,10649,4483,11424,43845,51309,33444,51218,677,2493,676,11660,10607,675,10658,47637,48771,26287,51145,12973,4206,9614,1704,40377,3566,4507,47223,51533,10304,50742,51255,51292,1338,681,38380,50100,21519,499,5903,10264,10084,51138,51283,51293,51362,2670,44793,17613,4525,34586,1547,9751,21574,10872,51274,9667,3324,51137,13870,21656,9726,39244,44476,40494,28024,2959,9639,51351,17629,2309,43015,38890,47690,51289,2078,51186,51273,45710,11326,35862,51260,50667,34604,9690,8943,10704,11287,11851,27953,31635,26498,1642,665,4275,35597,10900,51209,41746,51361,49238,38301,3337,42616,3204,35096,51248,12495,51244,1945,11176,19572,51236,51207,41392,656,9620,51267,51363,51415,9762,2479,51237,48194,51305,34575,786,2478,42540,13031,50139,6051,48417,3562,41502,6048,51286,11797,39610,51360,45859,10061,1980,51300,17388,43069,649,11321,1652,9760,22016,27014,51135,10752,4229,28008,51386,17125,645,1725,643,11501,18762,51331,33229,27671,51234,11270,1302,22076,15266,34154,980,4137,51457,51143,11425,8815,37397,50796,43834,40342,6102,51340,1987,32552,51167,3372,39750,3361,15269,3628,17222,22196,51208,51223,4249,38136,10889,3603,47495,47602,51310,48869,39130,11349,36480,50823,635,33240,1300,50303,22226,51397,51156,33563,43442,1695,2949,51221,4659,406,1581,50708,394,51247,391,9645,51307,51195,24192,51061,9739,51153,43949,5449,39705,628,29041,625,17119,6181,11354,4691,11986,9665,1337,1228,912,51008,22461,26238,8679,17260,8685,8674,2168,12412,4702,3922,51323,8665,2281,44932,8661,34882,1040,794,17093,51316,51366,45466,51297,49293,4711,4079,9778,9559,22565,41506,51329,41068,42692,50612,9638,11005,906,48196,50642,9622,577,51390,34186,17702,51261,12453,45588,586,47535,596,595,34174,51225,51256,51270,51326,51356,590,592,3258,1683,2025,8592,17428,51227,50736,11628,27234,48617,593,48026,4087,9608,28257,383,3608,6304,11920,34669,27215,9754,39587,3640,10172,1459,48907,33537,39295,11335,10156,50785,27887,11134,36366,18552,3263,50986,22771,48303,8548,6327,51240,10234,45150,4058,14788,3902,34739,12342,11962,3131,9733,8526,48541,39236,51210,1437,9642,51243,15676,10934,17047,4804,50740,40532,37546,50090,6380,51334,51228,51423,15272,38876,8498,2253,22912,574,1927,51303,9515,33515,1333,8492,35324,51206,51230,15982,6398,2438,2063,10635,2764,32063,51157,4812,6390,43844,20562,4819,9741,45161,6400,4822,3990,567,9722,9589,987,51348,20296,40804,39760,13244,25775,8441,10228,42982,4096,50204,2832,49075,806,51152,51164,45613,23023,9534,2833,3126,11907,2427,2694,49282,1469,27646,1528,10611,51150,51291,38408,43253,11841,2563,51427,40580,560,48291,51159,38205,44779,9679,47328,9554,33158,8388,4850,4156,16245,2547,17630,47592,549,1560,51216,41298,51155,51226,41135,42621,558,556,2064,10666,33277,17084,3194,9604,15274,38625,51290,38027,48696,35739,2421,4866,51192,937,24772,50533,3642,505,43655,51183,3130,45106,12582,51161,9635,3120,8319,34956,44602,2628,47957,1865,25867,11873,51269,9720,51304,51383,2166,2414,10843,550,51198,2412,51355,1946,37501,37948,51215,51217,50789,8294,8293,39277,51302,51252,51371,42983,34840,14517,35772,37231,4898,11450,51418,3392,9721,3161,543,29569,8259,41101,32060,51154,17534,51233,51338,25378,539,6575,8157,45782,11427,6572,50436,8250,5412,33842,538,27080,11243,1841,2974,17416,45553,2030,2042,38328,19569,50741,51173,51296,25817,11788,29624,16364,8204,3741,51231,494,51333,17564,45185,8182,533,11342,23622,35627,23654,27784,1611,530,23660,26782,6636,3556,3701,25789,2755,9599,51197,32190,527,15164,1893,9587,17469,17711,6656,41213,51330,11780,3711,51287,39561,2561,11537,6668,51280,8135,2658,23738,51203,51271,10618,14189,51277,525,3234,508,4214,10052,11222,13820,50631,33360,8106,2941,35842,9531,51149,15275,11461,2947,51416,2390,2389,43010,43857,9533,18556,2179,4993,51294,522,6717,51074,34177,39626,17609,1499,3917,10669,29793,50485,40289,51325,8074,28090,8078,1763,11362,27553,17693,45278,9993,2384,51229,10310,51151,2394,51246,10917,50711,51148,42255,519,6759,3201,3615,38477,518,9745,25481,17407,11580,50566,6768,11938,40530,15050,43874,49280,38428,510,2372,11861,3127,9618,51212,47526,18480,50518,35806,8015,2374,10923,40651,3004,28069,11164,9625,10670,3305,9564,18074,826,506,51342,51298,39697,2854,51259,9656,36435,51214,51180,51163,9652,14180,43831,501,41295,43886,51339,45351,9562,9544,20374,24047,2484,51392,24133,490,5058,489,3320,37618,27884,487,51224,1468,13716,486,24208,10783,43978,11662,51337,45467,17261,16991,27955,51219,51266,31542,51200,51336,41323,6874,2233,45617,51238,17144,51222,9585,38449,661,51381,9685,40400,482,1438,7897,24373,50627,51184,43883,35807,34764,24397,43085,51204,51279,51332,9660,10929,24393,476,17117,47832,10814,51242,2361,18614,471,49256,11631,10688,41329,11614,6919,51275,51372,43309,43770,24431,472,7850,474,9669,7829,5121,14531,10288,51190,11870,10734,51284,51196,51272,1545,462,3924,51170,51341,38115,3601,51308,19687,6975,6978,51374,11906,51205,51319,1423,1646,51504,34850,16350,3498,9675,51320,19216,10938,51343,39945,51187,51306,450,51285,3480,445,38674,2551,24769,448,20411,34727,3140,17442,442,9621,50144,17468,2242,38407,42122,9592,50818,51257,51344,5193,14530,2356,1996,7681,39752,9661,47544,27694,3005,836,10624,44366,33168,1484,51080,7662,51312,10318,51088,5208,51220,51299,1760,33967,4152,7104,51378,14053,51346,14172,11697,15259,33237,44421,11799,41720,10625,50791,51146,17050,1023,747,11887,2610,13684,16971,51396,45440,9586,3964,48887,51268,35101,7597,9952,37628,9650,3112,41317,51353,47551,437,1369,2326,43315,25118,3025,34569,12716,51301,51311,45507,20481,426,48253,2926,12025,45481,2329,50502,7550,47349,51358,5240,14862,7520,9664,416,3659,422,9541,2973,3323,423,444,2351,425,33481,3511,428,39774,415,19196,43007,51379,11002,51377,3134,51328,43849,24962,3280,417,45441,2883,418,51335,51398,1998,879,2633,9627,1357,51121,51254,3486,17032,38391,17734,9619,30419,51162,51253,17406,410,9734,408,51384,11790,2184,36569,17504,39658,409,11865,4314,2922,34690,45629,15088,25388,401,2112,51160,20809,35318,3610,11802,45350,51201,50714,17404,9647,11830,7264,50412,51282,2638,5284,33297,35024,49151,51245,3546,9693,34843,9583,17030,48799,51313,12584,31297,3362,51101,3399,47231,35094,51235,48607,51129,388,7322,51347,7331,41437,9996,51241,51318,34758,40591,43320,31336 )    
                    
GROUP BY contact_a.id 

LIMIT 0, 100000

This is while searching for contacts with activity type = x on advanced search and then export.

@seamuslee001
Copy link
Contributor Author

so @jitendrapurohit @eileenmcnaughton

For the Australian Greens the situation is / was this :

Prior:
All Primary Fields exports had been disabled via an extension
Loading the Select fields for Export screen would take ages and slow the db down and generally not actually Succeed

After:
Primary fields still disabled
Select fields for Export Screen loads in a reasonable time

I would say given your report @jitendrapurohit that the time taken for the select fields is improved with this PR I would suggest merging this

ping @andrew-cormick-dockery @johntwyman

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Jun 2, 2020

@colemanw any thoughts? I would note that the preview seems to be called in preProcess which cases it to be called an extra time on postProcess

Note @jitendrapurohit's query is actually the full query - not the preview query

@seamuslee001
Copy link
Contributor Author

The preview issue only came up since we merged in the new ExportUI. Its probably not hitting heaps of sites but just the ones where groups are a real performance point

…r the usage in preview data to speed up loading of the select fields screen
@seamuslee001 seamuslee001 changed the base branch from master to 5.26 June 2, 2020 23:12
@civibot civibot bot added 5.26 and removed master labels Jun 2, 2020
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton as discussed I have now put this against 5.26

@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

3 similar comments
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@seamuslee001
Copy link
Contributor Author

Test fail unrelated

I am going to merge this based on @jitendrapurohit 's testing showing that this improves performance on the select fields for export screen which is what this is targeting and that discussion with Eileen https://chat.civicrm.org/civicrm/pl/aytebtxgti8y5851jkcrmdu1fa says that she is fine for this to be merged into the RC

Comment on lines +151 to +155
foreach ($defaultProperties as $key => $var) {
if (in_array($key, ['groups', 'tags', 'notes'])) {
unset($defaultProperties[$key]);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the purpose of this foreach loop. You are just unsetting 3 keys from an array. FYI unset() does not raise any error if the key isn't there. I think this could be simplified to:

Suggested change
foreach ($defaultProperties as $key => $var) {
if (in_array($key, ['groups', 'tags', 'notes'])) {
unset($defaultProperties[$key]);
}
}
CRM_Utils_Array::remove($defaultProperties, 'groups', 'tags', 'notes');

@eileenmcnaughton
Copy link
Contributor

@colemanw note we should re-visit this in master - I felt we needed a quick fix for now but I think at least a place holder should be present

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants