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

[BUG?] QueryBuilder::setParameter does not exist, so it's not possible to add them during query building #789

Open
esserj opened this issue Dec 4, 2018 · 1 comment

Comments

@esserj
Copy link
Contributor

esserj commented Dec 4, 2018

Im going through the documentation of the query builder to find how I must deal with assigning parameters during the query build process.

this is what I come across: https://www.doctrine-project.org/projects/doctrine-phpcr-odm/en/latest/reference/query-builder-reference.html#gt-parameter

setParameter however is not implemented in the QueryBuilder. Was this removed for some reason?

Having such functionality imo is crucial as once you request the query to assign parameters that should apply you can no longer continue building. This would mean that any abstractions you implement for building would have to somehow track all parameters themselves and then assign it to the query, which makes me believe thats the querybuilders job?

On first glance it would also be fairly easy to implement, just assign them during Converter::getQuery() e.g. ConverterPhpcr:getQuery just after creation of the query.

This would not break BC i think, although it could be that some would have extended and implemented get/set parameters for their custom converter ...

any thoughts?

@dbu
Copy link
Member

dbu commented Jan 7, 2019

sorry that this issue went under. i now looked at it, and indeed it looks a lot like we did not finish adapting the query builder from the ORM. looking at the ORM query builder, it does as you propose (except its all in line and does not use a separate Converter).

i agree with you, the parameter methods to the phpcr query builder and making the converter apply them to the query would be a good improvement, and would not be a BC break. do you want to do a pull request for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants