Split Adapter::query() into prepareQuery() and executeQuery() - #172
Merged
simon-mundy merged 6 commits intoAug 12, 2026
Conversation
Adapter::query() conflated preparing a statement and executing SQL behind a stringly-typed second argument and a three-way union return type. prepareQuery() always prepares (and binds if parameters are given) without executing; executeQuery() executes raw SQL or a prepared statement and returns a deterministic Driver\ResultInterface. query() is now deprecated and proxies to the two new methods for backwards compatibility. AdapterInterface is left untouched to avoid breaking external implementors mid-0.x. Also adds Driver\ResultInterface::getQueryResult(), which clones and initializes a ResultSet from a query result, replacing the clone/initialize logic previously duplicated in Adapter.
simon-mundy
added
enhancement
The docblock claimed array{0: string, 1: string}, but the first
element is genuinely nullable for table-less selects (e.g. selecting
bare function expressions with no FROM clause), a case already
covered by SelectTest. A newer PHPStan release trusts the inaccurate
annotation and flags the isset() null-check in processSelect() as
dead code; correcting the annotation fixes the false positive without
touching runtime behavior.
Supersedes the RFC's original decision to leave AdapterInterface untouched for 0.x BC. External implementors will need to add both methods to remain compatible.
Collapses the mode-detection branching into a single match expression and a guard-clause early return for the prepare-only case, relying on the declared parameter type to distinguish array/ParameterContainer from an invalid string without extra type checks.
The message only needs to flag that the mode string was wrong; it doesn't need to enumerate the other accepted parameter types.
AdapterInterface declared string|Driver\StatementInterface, putting the scalar before the object type and diverging from the codebase's convention (and from Adapter's own implementation signature).
tyrsson
approved these changes
Aug 12, 2026
tyrsson
left a comment
tyrsson
left a comment
Member
There was a problem hiding this comment.
Approved pending all test are passing (unit / integration). As long as test are passing local as I know there is issues with action runs prior to the mago migration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #171: splits
Adapter::query()into two single-purpose methods, deprecatingquery()as a BC-preserving proxy.prepareQuery(string $sql, ParameterContainer|array $parameters = []): Driver\StatementInterface— always prepares (and binds if parameters are given), never executes.executeQuery(string|Driver\StatementInterface $sql): Driver\ResultInterface— executes raw SQL or a prepared statement and returns a deterministicDriver\ResultInterface. Callers checkisQueryResult()and callgetQueryResult()themselves if they want aResultSet.query()is now@deprecated, proxying to the two new methods internally, preserving all prior behavior.AdapterInterfacenow also declaresprepareQuery()andexecuteQuery(). This supersedes the RFC's original BC-avoidance stance of landing the new methods onAdapteronly — external implementors ofAdapterInterfacewill need to add both methods to stay compatible.Driver\ResultInterface::getQueryResult(?ResultSetInterface $resultPrototype = null): ResultSetInterface, implemented inPdo\Result, replacing the clone/initialize logic previously duplicated inAdapter.Test plan
Added coverage for
prepareQuery(),executeQuery(), andgetQueryResult(); updated existingquery()tests for the new delegated architecture