Fix deprecated DBAL calls #8794
Conversation
greg0ire
force-pushed
the
improvement/dbal-depecations
branch
from
a251249 to
ddd921f
Compare
greg0ire
force-pushed
the
improvement/dbal-depecations
branch
from
ddd921f to
afa837a
Compare
|
Thanks @derrabus ! |
| @@ -74,7 +74,7 @@ public function generate(EntityManager $em, $entity) | |||
| $sql = $conn->getDatabasePlatform()->getSequenceNextValSQL($this->_sequenceName); | |||
|
|
|||
| // Using `query` to force usage of the master server in MasterSlaveConnection | |||
| $this->_nextValue = (int) $conn->query($sql)->fetchColumn(); | |||
| $this->_nextValue = (int) $conn->executeQuery($sql)->fetchOne(); | |||
There was a problem hiding this comment.
this change made the ORM incompatible with the PrimaryReadReplicaConnection (the new name of MasterSlaveConnection) by not respecting the comment just above it. executeQuery is the API where reads are allowed from the read replica
The alternative is to force connecting to the master here instead.
There was a problem hiding this comment.
Can your provide a PR to fix that? I don't use PrimaryReadReplicaConnection anywhere at the moment.
There was a problem hiding this comment.
I started a Slack discussion about that, to see what is the right fix for that.
Semantically, we can have 3 kind of execution:
- read-only queries, for which we want the result ->
executeQuery, andPrimaryReadReplicaConnectionlet them go through the read replica (if not already connected to the primary) - write statements without a result (e.g.
UPDATEfor instance) ->executeStatement(returning the number of affected rows), , andPrimaryReadReplicaConnectionautomatically ensures that they go to the primary - statements needing write priviledges but returning a result (e.g. moving to the next value of a sequence like here) ->
queryandPrimaryReadReplicaConnectionensures that they go to the primary
The issue here is that query got deprecated in favor of executeQuery, meaning that we don't actually have a method for the third case anymore (which would force to add support for PrimaryReadReplicaConnection in each place needing that kind of query). To me, the right fix would be to undeprecate query (or add a new method name for that behavior, using the new return type) in DBAL, and then using it in the ORM
This PR fixes some calls to deprecated DBAL methods.