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

Improve Comparison::CONTAINS: allow to use the SQL wildcard characters #45

Open
wants to merge 1 commit into
base: 2.0.x
Choose a base branch
from

Conversation

Fedik
Copy link

@Fedik Fedik commented Oct 25, 2014

This pull additional to doctrine/orm#1150
And allow to use the SQL wildcard characters % and _ for Comparison::CONTAINS

Allow to build the contains expressions like:

Criteria::expr()->contains('myField', 'some string%');
Criteria::expr()->contains('myField', '%some string');
Criteria::expr()->contains('myField', '%10\%'); // search for 10% at end of the string
Criteria::expr()->contains('myField', 's_m_ string');

Note: this changes can make b/c issue for existing Comparison::CONTAINS expressions, when someone use not escaped %, _ .

@stof
Copy link
Member

stof commented Oct 27, 2014

I'm -1 for this:

  • it makes the API harder to use
  • it is a BC break

If we want wilcard support, the only way is to uuse a new operator. We cannot change existing ones

@Fedik
Copy link
Author

Fedik commented Oct 27, 2014

I agree, that a new operator would be a better solution,
but my knowledge not enough to do that, so I did what I can ...

@Ocramius
Copy link
Member

This is probably an actual like/ilike operator to be introduced

@@ -152,7 +152,28 @@ public function walkComparison(Comparison $comparison)

case Comparison::CONTAINS:
return function ($object) use ($field, $value) {
return false !== strpos(ClosureExpressionVisitor::getObjectFieldValue($object, $field), $value);
$field_value = ClosureExpressionVisitor::getObjectFieldValue($object, $field);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is uncovered by tests

@Fedik
Copy link
Author

Fedik commented Jan 22, 2015

@Ocramius I think if like/notlike #50 will be accepted then current issue can be closed ...
I keep it open as get not to much feedback about like/notlike

@stof
Copy link
Member

stof commented Jan 22, 2015

The current issue cannot be closed even if #50 is accepted. this PR is about fixing a bug in the current implementation, where the SQL generated for CONTAINS is not consistent with the behavior of Doctrine Common.

@Ocramius
Copy link
Member

Thanks for clearing that up, @stof - missed that part completely while
responding to the issue today.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 22 January 2015 at 11:38, Christophe Coevoet [email protected]
wrote:

The current issue cannot be closed even if #50
#50 is accepted. this PR is
about fixing a bug in the current implementation, where the SQL
generated for CONTAINS is not consistent with the behavior of Doctrine
Common.


Reply to this email directly or view it on GitHub
#45 (comment).

@Fedik
Copy link
Author

Fedik commented Jan 22, 2015

@stof I thought doctrine/orm#1150 about SQL .. (where you told about unescaped value)

@Ocramius Ocramius self-assigned this Apr 14, 2015
Base automatically changed from master to 2.0.x January 19, 2021 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants