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.

Sign up for GitHub

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

Jump to bottom

[proposal] Add iLike support #1564

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

[proposal] Add iLike support #1564

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4bbf00a
add ILIKE to where constraint
gstreetmedia May 30, 2018
79e7494
Merge pull request #1 from gstreetmedia/ilike-support-patch
gstreetmedia May 30, 2018
6447fa8
fix formatting issue
gstreetmedia May 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/waterline/utils/query/private/normalize-constraint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -784,7 +785,7 @@ module.exports = function normalizeConstraint (constraintRhs, constraintTarget,
// ╦ ╦╦╔═╔═╗
// ║ ║╠╩╗║╣
// ╩═╝╩╩ ╩╚═╝
else if (modifierKind === 'like') {
else if (modifierKind === 'like' || modifierKind === 'ilike') {

// If it matches a known attribute, verify that the attribute
// does not declare itself `type: 'boolean'` or `type: 'number'`;
Expand Down