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-21531 - replace regex with POSIX-compliant alternative for MariaD… #11388

Merged
merged 1 commit into from
Dec 8, 2017

Conversation

MegaphoneJon
Copy link
Contributor

…B compatibility

Overview

MySQL and MariaDB use different regex libraries. Certain non-POSIX regex statements will cause MariaDB to fail to execute a query, crashing CiviCRM. See https://jira.mariadb.org/browse/MDEV-7127 for details.

Before

Searching for a multi-select custom field value that contained a parenthesis would succeed on MySQL and fails on MariaDB with the error: "POSIX collating elements are not supported".

After

The search succeeds on both MySQL and MariaDB.

@seamuslee001
Copy link
Contributor

@eileenmcnaughton @monishdeb i feel like both of you have had some experience with this stuff

@eileenmcnaughton
Copy link
Contributor

Yeah - pretty sure that line has bounced back & forwards a few times :-) - perhaps git blame will provide the reasons

@totten
Copy link
Member

totten commented Dec 8, 2017

The git history suggests that @jitendrapurohit or @monishdeb might have some thoughts on the regex in this file.

FWIW, code/explanation seems reasonable. @MegaphoneJon , it sounds like you've r-run it on a couple different DBMS builds?

@monishdeb
Copy link
Member

To ensure its safe to merge I tested against MySQL and it worked for me. I am happy with this patch, merging now :)

Referred https://mariadb.com/kb/en/library/regular-expressions-overview/#

@monishdeb monishdeb merged commit 91b706b into civicrm:master Dec 8, 2017
sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-21531 - replace regex with POSIX-compliant alternative for MariaD…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants