-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix for incorrect query building in Repositories. #691
Conversation
$queryBuilder->andWhere($this->getPropertyName($property) . ' = :' . $property); | ||
} else { | ||
$queryBuilder->andWhere($this->getPropertyName($property) . ' IN (:' . $property . ')'); | ||
} |
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.
I would replace it with Doctrine Expr
class calls:
if (!is_array($value)) {
$queryBuilder->andWhere($queryBuilder->expr()->eq($this->getPropertyName($property), $property));
} else {
$queryBuilder->andWhere($queryBuilder->expr()->in($this->getPropertyName($property), array($property)));
}
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.
Great idea. I forgot about Expr
. Thanks
@Arn0d I agree. What about adding constants with operators from the WHERE clause to ping @pjedrzejewski @stloyd |
@Arn0d When I started to implement this i realized that the idea with constants is not the best. Because in the case when $value is array we can want to create complicated query like |
@pjedrzejewski Please merge. We are going to QA tonight. Thats why i need this fixed. |
@molchanoviv This can't be merge till Travis-CI will pass, now it dies with Doctrine exception, please fix it and then we can merge. |
@molchanoviv : ok ! |
if (!is_array($value)) { | ||
$queryBuilder->andWhere($queryBuilder->expr()->eq($this->getPropertyName($property), ':' . $property)); | ||
} else { | ||
$queryBuilder->andWhere($queryBuilder->expr()->in($this->getPropertyName($property), ':' . $property)); |
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 is not correct, it must be array(':'.$property)
otherwise Doctrine will fail.
fix for incorrect query building in Repositories.
Thanks Ivan! |
This broke the spec: |
fix for incorrect query building in Repositories If
$value
is array.