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

Remove deprecated methods from the wrapper Connection #4167

Merged
merged 6 commits into from
Jul 21, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Jul 12, 2020

Q A
Type improvement
BC Break yes

Summary

  1. Calls like $conn->query()->fetch*() have been reworked to $conn->fetch*() before the method removal.
  2. The rest of the calls to $conn->query() have been replaced with $conn->executeQuery() and $conn->executeStatement() depending on whether the statement yields rows.
  3. Calls to $conn->exec() and $conn->executeUpdate() have been replaced with $conn->executeStatement().
  4. The overridden versions of the insert(), update() and delete() methods have been removed form PrimaryReplicaConnection since they all are implemented via executeStatement() in the parent class which ensures connection to the primary instance.

TODO:

  • Unlike PrimaryReplicaConnection::query(), PrimaryReplicaConnection::executeQuery() doesn't automatically connect to the primary instance. I have no idea why the method semantics are different in the first place. It looks like a design flaw to me. This should be reflected in the deprecation note in 2.11.x.

    Note that PrimaryReplicaConnection::query() ensures connection to the primary instance while executeQuery() doesn't.

    Depending on the desired behavior:

    • If the statement doesn't have to be executed on the primary instance, use executeQuery().
    • If the statement has to be executed on the primary instance and yields rows (e.g. SELECT), prepend executeQuery() with ensureConnectedToPrimary().
    • Otherwise, use executeStatement().

    Updated the upgrade notes in Additional deprecation note for PrimaryReplicaConnection::query() #4175.

@morozov morozov force-pushed the remove-wrapper-query-exec branch from a5b0eff to ea020aa Compare July 12, 2020 15:08
@morozov morozov requested a review from greg0ire July 12, 2020 15:20
@morozov
Copy link
Member Author

morozov commented Jul 15, 2020

@greg0ire could you take a look please?

@greg0ire greg0ire changed the title Remove deprecated methods form the wrapper Connection Remove deprecated methods from the wrapper Connection Jul 21, 2020
@greg0ire
Copy link
Member

Sure! Your message ends with three backticks, did you maybe mean to write more?

@greg0ire
Copy link
Member

Also, do you mean to fix the todo in another PR?

@morozov
Copy link
Member Author

morozov commented Jul 21, 2020

Your message ends with three backticks, did you maybe mean to write more?

No, it was a code block originally.

Also, do you mean to fix the todo in another PR?

Yes, it will have to be done in 2.11.x.

@morozov morozov merged commit 4d17281 into doctrine:3.0.x Jul 21, 2020
@morozov morozov deleted the remove-wrapper-query-exec branch July 21, 2020 19:22
@morozov morozov self-assigned this Jul 23, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants