-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add hints to QueryBuilder #12268
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 hints to QueryBuilder #12268
Conversation
|
There was also an attempt to contribute this before: #6359 |
greg0ire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, although maybe you should contribute docs to https://github.com/doctrine/orm/blob/3.5.x/docs/en/reference/query-builder.rst explaining use cases for this method.
|
I did not see the previous PR. I will have a look and try to add documentation, thanks for the hint. |
85cb9fb to
08a6937
Compare
|
I am sorry to bother again but I showed this PR to my colleagues and they also did an internal review. One suggestion was to mirror the functionalities of the Query class and also provide the |
|
I agree with the renaming. Regarding |
|
The //geHint example
$hint = null;
// $qb instanceof QueryBuilder
$hints = $qb->getHints();
if (array_key_exists('name', $hints)) {
$hint = $hints['name'];
}
//vs
$hint = $qb->getHint('name');
//hasHint example
$hints = $qb->getHints();
if (!array_key_exists('name', $hints)) {
$qb->setHint('name', 'value');
}
//vs
if (!$qb->hasHint('name')) {
$qb->setHint('name', 'value');
} |
|
Ok, but what is the benefit for your application specifically? |
|
Our very specific case is that we built an adapter to have multiple custom SqlOutputWalkers for e.g. permissions and translations. In combination with multiple Form Extensions where we only have the QueryBuilder available we want to check if a certain hint was already set and act accordingly. |
08a6937 to
2504e97
Compare
2504e97 to
a7c5b17
Compare
…o the query This adds the membervariable hints to the QueryBuilder to enable setting hints that will be applied to the query when it is created. This can help trigger custom walker classes when the query is not adressable driectly e.g. in Symfony Form Extensions where the quer_builder normalizer is handed the querybuilder directly. Also see doctrine#11849 The feature mirrors the hint feature from the Query class. This also adds tests for the hints in the QueryBuilder to ensure that those are added correctly and applied to the query itself on creation
a7c5b17 to
cdd7749
Compare
|
I rebased my branch to the current 3.6.x version but the pipeline fail looked unrelated to my changes. |
|
Thanks @pmaasz ! |
Context
Doctrine offers with the Query class to add hints to the query which open the door to some more features regarding customizing queries. Currently hints can only be set on the query itself which covers 99% of cases to utilize that feature but in some cases only the QueryBuilder is available.
The problem
When working with a custom SqlOutputWalker a hint needs to be set on the query to trigger the walker and apply the logic to the query. In some instances that is impossible like e.g. when working with Symfony Form Extensions where only the QueryBuilder is available and yes you could also call get query but prebuilding the query is not always an option because of a lot of side effects and other sql manipulation.
Other devs also mentioned this option as a nice to have in #11849 or symfony #37582.
Possible solution
In this PR I explore the implementation of a hint param in the QueryBuilder to hold hints that should be applied to the query when it is created.
I am unsure what I could possibly break and hope that this suggestion at least opens up the discussion to widen the possibilities of hints.