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-17748: Expose options to CRM_Core_DAO #10585

Merged
merged 1 commit into from
Aug 29, 2017

Conversation

monishdeb
Copy link
Member

@monishdeb monishdeb commented Jun 29, 2017

@lcdservices
Copy link
Contributor

this looks good. ran a test running the same query through executeQuery and executeUnbufferedQuery and confirmed the unbuffered format was working as expected.

@monishdeb monishdeb changed the title CRM-17748: Expose options to CRM_Core_DAO [ready for core team review] CRM-17748: Expose options to CRM_Core_DAO Jul 6, 2017
@seamuslee001
Copy link
Contributor

@eileenmcnaughton I think this makes sense, code looks sensible, Brian appears to have tested it and confirmed working

@eileenmcnaughton
Copy link
Contributor

I'm pretty much ready to merge based on the combined review of @seamuslee001 & @lcdservices + seeing that the code looks good & appears to only impact when a specific, not normal, path is followed. I do recall last time we tried to expand this code there was a problem with exports - but I think that was only because I tried to make it call executeUnbufferedQuery from the export function & this doesn't. @monishdeb do you remember?

@monishdeb
Copy link
Member Author

@eileenmcnaughton yes and that's still an issue for export, where we cannot use unbufferedQuery. I stated the reason here

@eileenmcnaughton
Copy link
Contributor

@monishdeb but nothing in this pr gives you pause?

@monishdeb
Copy link
Member Author

monishdeb commented Aug 29, 2017

Yes. The patch just exposes the underlying DB options as listed here And unbuffered queries still work for single (NOT nested) queries.

@eileenmcnaughton
Copy link
Contributor

cool - merging

@eileenmcnaughton eileenmcnaughton merged commit 33754fd into civicrm:master Aug 29, 2017
@eileenmcnaughton eileenmcnaughton changed the title [ready for core team review] CRM-17748: Expose options to CRM_Core_DAO CRM-17748: Expose options to CRM_Core_DAO Aug 29, 2017
@seamuslee001 seamuslee001 deleted the CRM-17748 branch June 24, 2020 21:10
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.

5 participants