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

[proposal] Add iLike support #1564

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gstreetmedia
Copy link

@gstreetmedia gstreetmedia commented May 30, 2018

This small change provides postgres support for ILIKE alongside LIKE. The "contains", "startsWith" and "endsWith" contraints only support "LIKE" And while it appears that the direct use of "LIKE" is supported (although undocumented) there is no equivalent pass-thru support for ILIKE.

I'd like to see this support flow over to mySQL, but that would have to come in the form of

COLLATE UTF8_GENERAL_CI like '%$search%'

Or something similar.

Also need to figure out why this affects populating associations.

A query with an association using like might end up as

"where": { "members.name": { "like": "%greenstreet%" } },

where as a query with an association using ilike will end up like

"where": { "members.name": { "members.ilike": "%greenstreet%" } },

I have yet to track down where this happens. Somehow the query building is smart enough to detect the presence of "like" but not "ilike"

@sailsbot
Copy link

Hi @gstreetmedia! It looks like your pull request title doesn’t quite conform to our guidelines. Please edit the title so that it starts with [proposal], [patch], [fixes #], or [implements #]. Once you've fixed it, post a comment below (e.g. "ok, fixed!") and we'll take a look!

@gstreetmedia gstreetmedia changed the title Ilike issue 1563 [proposal] Add iLike support May 30, 2018
@@ -44,6 +44,7 @@ var MODIFIER_KINDS = {
'in': true,

'like': true,
'ilike': true,
'contains': true,
'startsWith': true,
'endsWith': true
Copy link
Member

Choose a reason for hiding this comment

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

The lay of the land right now is that the constraint modifiers like like, in, etc are (for now) standard, across all adapters, and currently, case-sensitivity of lookup behavior varies based on the database -- that was a Waterline 0.13 change we made to allow for better performance. That did come at the cost of interoperability, and someday I'd like to solve that problem more elegantly.

@gstreetmedia But for the immediate term, I'm gathering you have a need to do case insensitive lookups on PostgreSQL, which totally makes sense, so focusing in on that for a sec: Seems like there's two options:

  1. Make ilike a formal part of the Waterline adapter spec and implement it across the board. In some databases/charsets (like default MySQL), I think it'll be exactly the same thing as like. A lot of the work for that is going to be in Waterline core. It also involves updating docs, and possibly also waterline-utils. We'd probably also want to update the other core adapters, including sails-mongo -- if not to support this, then at least to throw an error indicating that it's not supported in that adapter. (hard)
  2. Implement support for a meta key in sails-postgresql that makes all LIKE modifiers case-insensitive (easy)

My gut says that we should only bother messing with the second option right now. It's less powerful, since it means you don't have granular control over the case-sensitivity of the various LIKE modifiers in the query (it's all or nothing). This would only involve changing sails-postgresql, and I believe it could be done by recursively parsing the criteria ahead of time, transforming like into ilike

P.S. By the way, we shouldn't even have startsWith, endsWith, and contains in here -- they're normalized ahead of time in Waterline core. I suspect that @particlebanana either added them back here because of a few places in the join logic where we were previously calling out directly to adapter methods without first forging the query. That should be resolved now so we can probably remove them -- although they're not exactly hurting anything.

@sqcrh
Copy link

sqcrh commented Oct 17, 2019

@mikermcneil Looking through waterline and the supporting packages, I think another more flexible approach would be simply adding a configuration for waterline that white lists custom modifiers. In the case of SQL these modifiers just get forwarded to knex. That way if someone wants ilike or rlike they can tell waterline to let it through explicitly with a configuration and it won't be an issue for anyone else that doesn't use those. I would be happy to start a pull request, for now we are just using this fork.

In regard to globally configuring case sensitivity, i think if we wanted that we could probably make a case insensitive index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants