-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fixed counting exception #1188
Fixed counting exception #1188
Conversation
Fixed "Invalid parameter number: number of bound variables does not match number of tokens " exception during execution count on Query where select part of query contains :parameters.
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-3404 We use Jira to track the state of pull requests and the versions they got |
Needs a test case. |
$parameters = $countQuery->getParameters(); | ||
foreach($parameters as $k=>$param){ | ||
if(!array_key_exists($param->getName(), $parameterMappings)){ | ||
$parameters->remove($k); |
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.
This seems wrong: too many bound parameters and you are removing some? This may even introduce security issues...
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.
Doctrine didn't removes it automatically when count is performed on something like this:
return $this->getEntityManager()->createQuery(
"select r,"
. "(sum(case when rr.helpful=true then 1 else 0 end)* 100 / (count(rr.helpful)+0.00001)) as hidden percent_cnt,"
. "(case when (sum(case when rr.helpful=true then 1 else 0 end)* 100 / (count(rr.helpful)+0.00001))>:percent_of_helpfulness and count(rr.helpful)>:minimum_votes then 1 else 0 end) as hidden valuable_cnt"
. " from MerixGameBundle:Review r left join r.reviewRatings rr"
. " where r.game = :game group by r.id order by valuable_cnt desc, percent_cnt desc"
)
->setParameter('percent_of_helpfulness', self::PERCENT_OF_HELPFULNESS)
->setParameter('minimum_votes', self::MINIMUM_VOTES)
->setParameter('game', $game);
Executing the same test many times causes error because AI fields.
@Ocramius Could You comment now?
|
@@ -151,6 +131,39 @@ public function count() | |||
return $this->count; | |||
} | |||
|
|||
public function getCountQuery(){ |
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.
Missing docblocks: also, please put the brace on a new line
Missing doc block, removed parse() parameter.
Corrected. |
Seems good to me. @asm89 got time to proof-read it? |
Pulling it locally, fixing minor CS and merging :-) |
Thanks :-) |
…()` private, as it is an implementation detail
…ery()` via reflection for test purposes
@merixstudio merged, thanks! |
@Ocramius We made that fix because our project working on 2.4, could you backport this code? |
@merixstudio can do |
…()` private, as it is an implementation detail
…ery()` via reflection for test purposes
…emoved-query-parts-2.4-backport' into 2.4 Merge #1188 into 2.4
Merged into 2.4 in c701e8b |
* | ||
* @return Query | ||
*/ | ||
public function getCountQuery() |
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.
this should be a private method. There is no reason to have it in the public API
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.
sorry, I just saw it was done when merging
Fixed "Invalid parameter number: number of bound variables does not match number of tokens " exception during execution count on Query where select part of query contains :parameters.