DDC-1536: GH-213: QueryBuilder::getQuery #1926

Closed
opened 2026-01-22 13:32:13 +01:00 by admin · 8 comments
Owner

Originally created by @doctrinebot on GitHub (Dec 13, 2011).

Originally assigned to: @guilhermeblanco on GitHub.

Jira issue originally created by user @beberlei:

Pull-Request was automatically synchronized: https://github.com/doctrine/doctrine2/pull/213

Hi there,
I'm wondering myself if the QueryBuilder::getQuery is a good method name.
Actually, a getMethodObject makes me think that I can use many times the getMethodObject and work with the same object.
e.g :

$qb->getQuery()->useResultCache (true);
$qb->getQuery()->execute();

In my code, the problem is that getQuery return a new instance of Query each time that I call getQuery.
Furthermore, the getQuery method invoke a createQuery method.

So, maybe the QueryBuiler::getQuery can store a reference to the created Query instance, and if the QueryBuilder::getQuery is call many times without any change return the stored instance.
If the QueryBuilder::getQuery have to generate a new Query, throw an exception or something like that.

Because of a change to QueryBuilder::createQuery is a major change, I don't think that It could be the solution, but it's a reflexion around this.

What do you think about that ?

Originally created by @doctrinebot on GitHub (Dec 13, 2011). Originally assigned to: @guilhermeblanco on GitHub. Jira issue originally created by user @beberlei: Pull-Request was automatically synchronized: https://github.com/doctrine/doctrine2/pull/213 Hi there, I'm wondering myself if the QueryBuilder::getQuery is a good method name. Actually, a getMethodObject makes me think that I can use many times the getMethodObject and work with the same object. e.g : ``` php $qb->getQuery()->useResultCache (true); $qb->getQuery()->execute(); ``` In my code, the problem is that getQuery return a new instance of Query each time that I call getQuery. Furthermore, the getQuery method invoke a createQuery method. So, maybe the QueryBuiler::getQuery can store a reference to the created Query instance, and if the QueryBuilder::getQuery is call many times without any change return the stored instance. If the QueryBuilder::getQuery have to generate a new Query, throw an exception or something like that. Because of a change to QueryBuilder::createQuery is a major change, I don't think that It could be the solution, but it's a reflexion around this. What do you think about that ?
admin added the Bug label 2026-01-22 13:32:13 +01:00
admin closed this issue 2026-01-22 13:32:13 +01:00
Author
Owner

@doctrinebot commented on GitHub (Dec 14, 2011):

Comment created by @guilhermeblanco:

This includes a BC break.
Marking as won't fix.

@doctrinebot commented on GitHub (Dec 14, 2011): Comment created by @guilhermeblanco: This includes a BC break. Marking as won't fix.
Author
Owner

@doctrinebot commented on GitHub (Dec 14, 2011):

Issue was closed with resolution "Won't Fix"

@doctrinebot commented on GitHub (Dec 14, 2011): Issue was closed with resolution "Won't Fix"
Author
Owner

@peterjmit commented on GitHub (Jan 15, 2016):

I would love to see this fixed in Doctrine 3. I am a long time user of doctrine (4+ years now) and a combination of the get prefix and the fluent interface of QueryBuilder led me to believe that getQuery would always return the same instance of Doctrine\ORM\Query.

A bad assumption to make, but one that resulted in a 2yr+ bug for us and a bit of a sharp edge!

@guilhermeblanco is this something you would consider revisiting for v3?

@peterjmit commented on GitHub (Jan 15, 2016): I would love to see this fixed in Doctrine 3. I am a long time user of doctrine (4+ years now) and a combination of the `get` prefix and the fluent interface of `QueryBuilder` led me to believe that `getQuery` would always return the same instance of `Doctrine\ORM\Query`. A bad assumption to make, but one that resulted in a 2yr+ bug for us and a bit of a sharp edge! @guilhermeblanco is this something you would consider revisiting for v3?
Author
Owner

@guilhermeblanco commented on GitHub (Jan 15, 2016):

There're a number of changes we're planning to do for v3. And yes, this is one of them. =)

@guilhermeblanco commented on GitHub (Jan 15, 2016): There're a number of changes we're planning to do for v3. And yes, this is one of them. =)
Author
Owner

@peterjmit commented on GitHub (Jan 15, 2016):

@guilhermeblanco brilliant, thanks 😄

@peterjmit commented on GitHub (Jan 15, 2016): @guilhermeblanco brilliant, thanks :smile:
Author
Owner

@Ocramius commented on GitHub (Jan 16, 2016):

This looks invalid to me btw: no interface can guarantee returning the same object over multiple calls (there is no way to make that type safe anyway).

The QueryBuilder already defies a lot of rules of good OO, but it's the nature of this object. getQuery is actually a cast operation, and doesn't cache the returned value (correctly, IMO).

@Ocramius commented on GitHub (Jan 16, 2016): This looks invalid to me btw: no interface can guarantee returning the same object over multiple calls (there is no way to make that type safe anyway). The QueryBuilder already defies a lot of rules of good OO, but it's the nature of this object. `getQuery` is actually a cast operation, and doesn't cache the returned value (correctly, IMO).
Author
Owner

@peterjmit commented on GitHub (Jan 16, 2016):

@Ocramius I understand your point and I don't necessarily think that getQuery should cache the result. I think this is less to do with OO/type safety, and more to do with DX/consistency.

Doctrine already has methods prefixed with create that return newly constructed objects, I think that signifies that the difference between get and create would be significant**, especially in this example where QueryBuilder::getQuery is a wrapper for EntityManagerInterface::createQuery.

** I mean specifically for methods that return objects

@peterjmit commented on GitHub (Jan 16, 2016): @Ocramius I understand your point and I don't necessarily think that `getQuery` should cache the result. I think this is less to do with OO/type safety, and more to do with DX/consistency. Doctrine already has methods prefixed with `create` that return newly constructed objects, I think that signifies that the difference between `get` and `create` would be significant**, especially in this example where `QueryBuilder::getQuery` is a wrapper for `EntityManagerInterface::createQuery`. *\* I mean specifically for methods that return objects
Author
Owner

@Ocramius commented on GitHub (Jan 16, 2016):

@peterjmit agree, we should probably move away from get* methods, and instead call it toQuery (or similar), which indeed expresses a completely different intent

@Ocramius commented on GitHub (Jan 16, 2016): @peterjmit agree, we should probably move away from `get*` methods, and instead call it `toQuery` (or similar), which indeed expresses a completely different intent
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#1926