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#1143 enable auto adding of backticks when doing an insert() or update() function #16193

Merged
merged 1 commit into from
Jan 3, 2020

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Jan 2, 2020

Overview

To support MySQL 8 there are a number of new reserved words such as grouping which need to be handled sensibly when inserting or updating or querying data. This change will when we do a create api call or similar that the DB_DataObject automatically adds backticks around the table field names and values which stops issues

Before

CRM_Core_BAO_MappingTest fails on MySQL8

After

CRM_Core_BAO-MappingTest passes on MySQL 8

ping @eileenmcnaughton @monishdeb @JoeMurray @totten

@civibot
Copy link

civibot bot commented Jan 2, 2020

(Standard links)

@civibot civibot bot added the master label Jan 2, 2020
@seamuslee001 seamuslee001 changed the title WIP enable auto adding of backticks enable auto adding of backticks when doing an insert() or update() function Jan 3, 2020
@seamuslee001 seamuslee001 changed the title enable auto adding of backticks when doing an insert() or update() function dev/core#1143 enable auto adding of backticks when doing an insert() or update() function Jan 3, 2020
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 this looks promising & probably we should have done it a long time ago given some bugs we've had around location table names in queryObject. Have you dug into how it actually works? The fact tests pass give me quite a bit of confidence....

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton basically what happens is that as far as i know for us is that when we call insert() or update() or delete() e,g, from https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/DAO.php#L543 that this https://github.com/civicrm/civicrm-packages/blob/master/DB/DataObject.php#L1074 is now true which then means it calls the quoteIdentifiers function in DB/mysqli.php here https://github.com/civicrm/civicrm-packages/blob/master/DB/DataObject.php#L1150 to wrap it in a backtick it also triggers quoting of the table name as well https://github.com/civicrm/civicrm-packages/blob/master/DB/DataObject.php#L1239

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 I'm inclined to this that so many tests go through here we can rely on that. Also this is at the start of the rc cycle so it gets 2 full months - I'm going to merge this

@eileenmcnaughton eileenmcnaughton merged commit 94af493 into civicrm:master Jan 3, 2020
@eileenmcnaughton eileenmcnaughton deleted the enable_backticks_dao branch January 3, 2020 22:03
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.

2 participants