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

CRM-20970 - avoid MySQL error caused by long numeric postal codes #10769

Merged
merged 1 commit into from
Jul 28, 2017

Conversation

mfb
Copy link
Contributor

@mfb mfb commented Jul 26, 2017

@eileenmcnaughton
Copy link
Contributor

I tried to replicate this, but couldn't. In my tests (via the test suite) I found another error in that line of code - if you search for postal code less than 5000 it includes contacts with no postal_code. If you are interested in testing & reviewing that change I will put up a PR.

I am going to merge this despite not being able to replicate the error. It seems that my mysql config is different, but I agree with the logic of not treating numbers longer than 10 characters as postal_codes and I agree that this patch does this.

I also noticed that this casting takes place regardless of the operator in use - there would be some performance cost to that in a '=' operation, perhaps not much.

@eileenmcnaughton eileenmcnaughton merged commit a7a62db into civicrm:master Jul 28, 2017
@mfb
Copy link
Contributor Author

mfb commented Jul 28, 2017

our production db server has STRICT_TRANS_TABLES - so my guess is that may be required to reproduce? casting an out of bounds integer is only a warning, but that changes to error when doing an insert with STRICT_TRANS_TABLES

I see that civicrm codebase has SET SESSION sql_mode = STRICT_TRANS_TABLES only if isDevelopment() which is identified by .svn or .git directory in the civicrm directory.

@mfb mfb deleted the CRM-20970 branch July 28, 2017 23:43
@mfb
Copy link
Contributor Author

mfb commented Jul 28, 2017

the error happens in CRM/Contact/BAO/GroupContactCache.php when creating temporary table 'civicrm_temp_group_contact_cache' . rand(0, 2000)

@xurizaemon
Copy link
Member

if isDevelopment() which is identified by .svn or .git directory in the civicrm directory.

I had never come across this! Behavior changes based on details of the hosting environment, wow

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