Dzoukr / Dapper.FSharp

Lightweight F# extension for StackOverflow Dapper with support for MSSQL, MySQL, PostgreSQL, and SQLite
MIT License
365 stars 35 forks source link

Support for ilike in postgresql #65

Closed kunjee17 closed 1 year ago

kunjee17 commented 1 year ago

Just like like there is something called ilike which is cooperation but case insensitive. It would be great if you can support that as well along side like

if you are Ok. I can give merge request.

Dzoukr commented 1 year ago

Hi my friend! I am for any improvement, but just thinking about how it would work with existing code - ilike is unknown for other DBs, isn't it?

kunjee17 commented 1 year ago

@Dzoukr you might be right. It is supported by Postgresql and MSSQL but not by others. So, it is all up to you if you like to put it there or not. Else I ll just fall back to good old dapper.

Dzoukr commented 1 year ago

I am just thinking about how to do it. It's always tricky to put something "only-somewhere-supported" into generic library. 🤔

kunjee17 commented 1 year ago

@Dzoukr you can give a skip if you want. It's all good. I totally understand to keep things simple.

kunjee17 commented 1 year ago

@Dzoukr just let me know. It is just once case where client wants that ilike should be there for comparison. Basically comparison should be case insensitive. If it can't be added with non-generic fashion then I ll close the issue.

JordanMarr commented 1 year ago

I think the way to do it would be to create Provider specific modules. For ex, MSSQL.ilike and Postgres.ilike. They could go next to the other filter functions: https://github.com/Dzoukr/Dapper.FSharp/blob/1a7382e830a26e67c14d4f94fc05c947bc931304/src/Dapper.FSharp/Builders.fs#L513

That way they wouldn’t be accidentally used for a Provider that doesn’t support them. Also, each provider’s name could vary if necessary. (For example, ms sql doesn’t refer to it as ilike.)

Dzoukr commented 1 year ago

I think the way to do it would be to create Provider specific modules.

Hmmm... That seems to be the way. But we would have to add another case here anyway, wouldn't we?

https://github.com/Dzoukr/Dapper.FSharp/blob/784001cad3a06f25fbad50592d8694f260e87531/src/Dapper.FSharp/Dapper.FSharp.fs#L18

kunjee17 commented 1 year ago

@Dzoukr yes we need to . That's why I told I can give quick PR. but as you said problem is. Postgresql and MSSQL are the only supporting that. So, how you bypass that here for eg. MySQL https://github.com/Dzoukr/Dapper.FSharp/blob/784001cad3a06f25fbad50592d8694f260e87531/src/Dapper.FSharp/MySQL.fs#L45 .

We can surely ignore ilike for like if that is Ok. Then we can surely add iLike support.

JordanMarr commented 1 year ago

ay. But we would have to add another case here anyway, wouldn't we?

Even easier would be to change it to this:

    | Like of caseInsensitive: bool * pattern: string
JordanMarr commented 1 year ago

One thing that it is a little confusing to me: I thought that SQL Server LIKE defaulted to be case insensitive, but apparently it is based on the column: https://stackoverflow.com/questions/14962419/is-the-like-operator-case-sensitive-with-sql-server

So I guess in terms of defaults, it would actually be inaccurate to say that SQL Server defaults to one or the other since it's not really controlled at that level.

This makes me think that your original idea of creating a separate DU case might be the better approach:

    | Like of string
    | ILike of string
module PostgresqlExtensions
 let ilike<'P> (prop: 'P) (pattern: string) = true 
Dzoukr commented 1 year ago

I thought that SQL Server LIKE defaulted to be case insensitive, but apparently it is based on the column:

In such a case it's out of library control, so I wouldn't bother... 😄

JordanMarr commented 1 year ago

Yeah, it's an edge case for sure. It makes me feel indifferent to adding it. Maybe a manual query is the way in this case (pardon the pun).

Dzoukr commented 1 year ago

Maybe a manual query is the way in this case (pardon the pun).

Yeah, I have the same feeling right now. It would be the first provider-specific thing on syntax level and I still find it somehow "dirty" (cannot explain why, just gut feeling)...

kunjee17 commented 1 year ago

I guess I agree with gentlemen here. @Dzoukr please do honor to close this issue.

Dzoukr commented 1 year ago

🤣 Honor to close the issue... God I love this community 😄 ❤️

Dzoukr commented 1 year ago

Adding this for v4 now

Dzoukr commented 1 year ago

Now available in version 4