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

Add optional $params argument to query/execute adapter methods #1962

Merged
merged 17 commits into from
Jun 17, 2021
Merged

Add optional $params argument to query/execute adapter methods #1962

merged 17 commits into from
Jun 17, 2021

Conversation

MasterOdin
Copy link
Member

@MasterOdin MasterOdin commented Feb 24, 2021

Closes #1954

This allows a new optional parameter to the query and execute adapter methods to allow you to pass in an array of parameters to be used in a prepared query on the underlying PDOAdapter. It's assume that custom adapters people may implement will also have some mechanism of utilizing a concept of prepared queries. To maintain backwards compatibility, if the optional parameter is empty, then we use the direct exec() and query() methods on the connection to maintain support for executing multiple queries in one go.

The aim here was to keep the return result for the two methods consistent with that's there, just adding the optional parameter. This is then also exposed upwards in the MigrationInterface and SeedInterface classes.

MasterOdin and others added 3 commits February 23, 2021 19:25
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
@MasterOdin MasterOdin changed the title Mpeveler feat query params Add optional $params argument to query/execute adapter methods Feb 24, 2021
@MasterOdin MasterOdin marked this pull request as draft February 24, 2021 00:33
@MasterOdin
Copy link
Member Author

This is marked as a draft as I've still got some amount of work to do to get this to work, and add tests across all adapters, but wanted input from @dereuromark, @garas, @Roy-Orbison that this would work for everyone's needs / expectations.

Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
@Roy-Orbison
Copy link
Contributor

This will cover most use cases.

All other problems/features I raised are voided by that fact that, when using the connection directly, one can call the startCommandTimer(), etc. methods themself. Exposing a wrapped connection or something to automate that timer would make the code unnecessarily complex.

MasterOdin and others added 5 commits March 28, 2021 13:12
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
@MasterOdin MasterOdin marked this pull request as ready for review June 17, 2021 11:49
@MasterOdin
Copy link
Member Author

@dereuromark alright, finally returned to this and got it working, and should be ready for review, with fixed test case, and have also added to the docs for this.

@dereuromark dereuromark added this to the 0.13 milestone Jun 17, 2021
@dereuromark dereuromark merged commit 7525393 into cakephp:0.next Jun 17, 2021
@MasterOdin MasterOdin deleted the mpeveler-feat-query-params branch June 17, 2021 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants