panter/ra-data-prisma

better case insensitive search

Closed this issue · 8 comments

macrozone commented

newest prisma supports "native" case insensitive search. Would be nice if we could use that if its avialable (can be checked through introspection), instead of the "poor mans" version that is currently in place

sMteX commented

I'd like to take a look at implementing this, now that some groundwork is done (#50 , thank you for merging!). I imagine it could be done in the same area (buildWhere.ts), somehow extending the String filter. What would you think would be a good idea as for how to opt-in into the insensitive mode? My initial guess was another suffix just for String filters, something like name_ci (=> comparator not found = default contains, and ci flag for case insensitive), but that can get quite convoluted if combined with explicitly stated comparator (surname_startsWith_ci). Would that be acceptable or do you have any other proposal? Of course ideally not breaking old functionality (and tests).

Also, are those functions accessible through different means than from Input components inside Filter? Just if we need to keep that in mind?

Also, related links

macrozone commented

@sMteX i would not opt-in, instead look into introspection if case insensitive is supported and use that if its available

sMteX commented

@macrozone

The new Prisma update added

enum QueryMode {
    default
    insensitive
}

which is present on StringFilter and StringNullableFilter (interestingly enough, not on their nested variants) under

input StringFilter {
   ...
   mode: QueryMode
   ...
}

both of those should be somewhere in introspection. So you would instead propose the default mode to be case insensitive, and opt-in into case sensitive (which would be mode : 'default') with something like _cs suffix (for case sensitive)? Also, the default mode corresponds to database collation, not guaranteed case sensitive.

macrozone commented

i would probably go for insensitive as thats what most will expect (consider a name search field).
We could introduce _cs, but i am not sure if its really needed.

i am open for suggestion

sMteX commented

I mean... if it's case insensitive, it would catch the case sensitive ones as well... I myself can't think of any use case where I would really need a case sensitive search (but then again, I'm not very experienced).

Ok, so to summarize:

  1. check introspection if we support case insensitive
    • if yes, use mode: insensitive by default
    • if not, don't use anything
  2. try to provide _cs if possible (i.e. not breaking any previous tests), but if it proved breaking, we could either think of something different or just not provide the choice at all (so it would be insensitive if supported, nothing if not supported)
sMteX commented

@macrozone

So while working on the implementation I realized that introducing another suffix (independent from the comparison) doubles the number of cases that can happen (does/doesn't have a comparison, does/doesn't have case sensitive flag), and each is slightly different (having to check if a given query is possible, and backtracking if not). It's not impossible but clutters the code with a lot of similar code that makes it a lot less readable in my opinion.

Do you think it's even worth it to support _cs suffix, or should we drop it and:

  • if used Prisma version supports QueryMode, always use insensitive
  • otherwise, use the default triple-case comparison?
macrozone commented

@macrozone

So while working on the implementation I realized that introducing another suffix (independent from the comparison) doubles the number of cases that can happen (does/doesn't have a comparison, does/doesn't have case sensitive flag), and each is slightly different (having to check if a given query is possible, and backtracking if not). It's not impossible but clutters the code with a lot of similar code that makes it a lot less readable in my opinion.
h
Do you think it's even worth it to support _cs suffix, or should we drop it and:

  • if used Prisma version supports QueryMode, always use insensitive
  • otherwise, use the default triple-case comparison?

yes, i would do that. Let the backend decide what its supported and make the frontend use case insensitive search if available and fallback to the current approach.

We still could add an option to the backend part to control that (whether it exposes QueryMode or not), but let's wait until someone actually raises this issue :-)

  • 👍1
macrozone commented

fixed in 4.2.0!

  • ❤️1