Feature/find in set expression

[hungthai1401]

Hi @tpetry ,
First, thanks for your package, it's very helpful for my working.
Sometime, I worked with FIND_IN_SET operator in MySQL so I want to contribute for this expression.
Plz take time to review it.

[tpetry]

Thanks for your contribution @hungthai1401. But it is not a good fit at the current implementation state.

The FIND_IN_SET function returns a match count for the search param. Only the MySQL implementation does that. So the meaning would have to be changed to something like StrListContains.

The FIND_IN_SET function would be case-insensitive and at least the PG one would be case sensitive. Maybe it would be a good approach to reimplement the functionality in the same way for all databases and let the user control the case sensitivity. But should it be case-sensitive or case-insensitive by default? Or has the sensitivity to be defined all the time?

The SQLite implementation is incorrect for these cases:

• The match is the first element
• The match is the last element
• The match is the only element

[tpetry]

Btw you should use JSON columns instead as they don‘t have a problem with values containing , for the values. Its the much better approach.

[hungthai1401]

Thanks for your suggestion @tpetry . I add strict mode in this expression for solve case-insensitive and case-sensitive.
I understand that is good if use JSON columns but as you know, some legacy projects didn't use JSON in schema

[hungthai1401]

Hello @tpetry , are you here?

[tpetry]

I'll need some time to refactor your PR.

[hungthai1401]

I'll need some time to refactor your PR.

thank you so much @tpetry

[tpetry]

I'll delay merging this PR until laravel/framework#46558 is merged. The current implementation of query expressions can't use arbitrary strings in expressions due to SQL injection problems. Only with the PR to Laravel this class will be of use.

[hungthai1401]

@tpetry I also follow your PR. Thank you for informing me

[hungthai1401]

@tpetry I can see that you PR has been merged

[tpetry]

Yes, but you'll have to wait for a Laravel release that includes the PR and a few days for me to include your PR. But it will be available soon ;)

[hungthai1401]

@tpetry I got it. thank you so much

[tpetry]

We'll have to wait another 2 weeks. My PR laravel/framework#47210 was merged shortly after the Laravel 10.13 release. We need that release to do this:

$query->where(new StringSetContains('type', new Value('blocked'))); 

I will polish your PR and prepare everything in the meantime. As soon as Laravel 10.14 is released I will then merge it.

[hungthai1401]

thanks for your info. I am very eager to use this syntax

[hungthai1401]

@tpetry Congratulation! Your PR has been released in v10.13.1

[tpetry]

This is now released with 0.5.0. Thanks for your contribution.

[hungthai1401]

@tpetry thanks for your great package.