DDC-3062: [GH-997] [FIX] Allow to use ManyToMany with all operators #3834

Closed
doctrinebot opened this Issue Mar 31, 2014 · 6 comments

2 participants

@doctrinebot

Jira issue originally created by user @doctrinebot:

This issue is created automatically through a Github pull request on behalf of bakura10:

Url: #997

Message:

Hi,

ping @guillhermoblanco : I think this may be blocking for 2.5

I introduced not so long ago support for ManyToMany for Criteria. However, I realized my implementation was really incomplete, because I hard-coded the "=" operator (https://github.com/doctrine/doctrine2/pull/885/files#diff-982b7374bbe9d5f4b6b71f4869a446eaR575). This means that it fails in a lot of cases when you use something different than "eq" for Criteria.

This PR fixes that, however it's a bit hacky. The SqlExpressionVisitor was made by type hinting for a BasicEntityPersister, preventing us from using us for a collection persister. Therefore I added a new interface to keep BC.

There is also a lot of code duplication (the whole "getSelectConditionSQL" was copied from the BasicEntityPersister), but without trait or BC, I have no idea about how to solve it.

All tests pass, test were added for other operators.

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-997] was assigned:
#997

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-997] was unassigned:
#997

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-997] was labeled:
#997

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-997] was labeled:
#997

@doctrinebot

Comment created by @doctrinebot:

A related Github Pull-Request [GH-997] was unlabeled:
#997

@doctrinebot

Issue was closed with resolution "Incomplete"

@Ocramius Ocramius was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment