Towards CRM-19815, CRM-19830 make pseudoconstant handling more generic in order to improve performance #9885
Conversation
1276fce to
f9fd891
Compare
|
I also grepped where CRM_Contact_BAO_Query::apiQuery is called from & the only place I could see where returnProperties might include prefix_id was tokens. I tested & found they used individual_prefix which is supported by virtue of being the machine name of the prefix_id option group. Also email greeting options etc look like Dear {contact.individual_prefix} {contact.first_name} {contact.last_name} |
f9fd891 to
796d3e8
Compare
| @@ -5829,12 +5811,40 @@ public function convertToPseudoNames(&$dao, $return = FALSE, $usedForAPI = FALSE | |||
| $val = $dao->{$value['idCol']}; | |||
| if ($key == 'groups') { | |||
| $dao->groups = $this->convertGroupIDStringToLabelString($dao, $val); | |||
| return; | |||
| continue; | |||
There was a problem hiding this comment.
@eileenmcnaughton That will be problematic in PHP7 PHPCompatibility/PHPCompatibility#292
There was a problem hiding this comment.
or at the very leaset maybe a false positive not sure i see it come up in phpcs on php7
There was a problem hiding this comment.
But, it's within a foreach loop - that seems to say 'outside a foreach loop'
foreach ($this->_pseudoConstantsSelect as $key => $value) {
There was a problem hiding this comment.
True didn't notice the foreach but have checked and seems ok but just something to watch out
|
@eileenmcnaughton fixed the test for you, looks to be just whitespace change in the AS part of the sql |
Using metadata pseudoconstants we can remove joins on the option_value table, which hur performance. This patch only opens it up within the context of the Contact table. It is intended to open the code up to allow us to extend the performance advantage of dropping the bad join to other entities. The following fields have metadata links to the option_value table: gender_id prefix_id suffix_id preferred_communication_method communication_style_id preferred_language The general pattern for the api is to return (eg) communication_style_id = 1, communication_style = 'Formal', where communication_style is the db name of the option group for communication_style_id. preferred_communication_method is the exception. For api calls it returns an array of ids like all other api calls. However, it returns a string of names for non-api calls on that field, allowing us to avoid handling it in a number of other places. I added tests to ensure no change on the api inputs & outputs and searched these fields through search builder & advanced search + export. I also checked profile search listings. They are not using the convert routine & have some e-notices that pre-existed, but no regression I could see.
47b4b19 to
86ab13b
Compare
CRM-19815, CRM-19830 make pseudoconstant handling more generic in order to improve performance
The point of this change is to open up option-value-based pseudoconstant handling from hard-coded fields to metadata based fields. Using pseudoconstants rather than hard-coding joins to the option value table removes slow joins and improves speed. In some cases I have been able to get 60 fold increases.
@seamuslee001 I am working through #9631 to feel extra sure about the components. I have done the following checks here.
I've added a bunch of api tests and done some search tests by trying the contact table option_value fields from search builder, advanced search & export. Profile search listings I tested too, the enotice pre-exists :-)