-
Notifications
You must be signed in to change notification settings - Fork 44
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
feat: Add case insensitive like
operator
#2368
feat: Add case insensitive like
operator
#2368
Conversation
c46458d
to
e9986f3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2368 +/- ##
===========================================
- Coverage 75.03% 75.02% -0.01%
===========================================
Files 266 268 +2
Lines 25855 25889 +34
===========================================
+ Hits 19400 19423 +23
- Misses 5150 5160 +10
- Partials 1305 1306 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! This is a nice usability feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thought(out-of-scope): _nilike
seems odd reading it, what if we prepend all negated labels with _n_
so for example we would have _n_eq
, _n_in
, _n_ilike
@@ -40,6 +40,10 @@ func matchWith(op string, conditions, data any) (bool, error) { | |||
return like(conditions, data) | |||
case "_nlike": | |||
return nlike(conditions, data) | |||
case "_ilike": | |||
return ilike(conditions, data) | |||
case "_nilike": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this here to keep under a thread:
From @shahzadlone
thought(out-of-scope): _nilike seems odd reading it, what if we prepend all negated labels with n so for example we would have _n_eq, _n_in, _n_ilike
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can see if others feel the same way but in my opinion, _n_...
make every negated labels ugly 😅 while _nilike
makes only this one ugly. But I'm open to the idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do prefer _nilike
over _n_ilike
at first glance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add a logical not operator to connor? Then we could remove the _n
variants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just that the first three chars read _nil
... haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add a logical not operator to connor? Then we could remove the _n variants.
Maybe like this?
switch op {
case "_and":
return and(conditions, data)
case "_eq":
return eq(conditions, data)
case "_ge":
return ge(conditions, data)
case "_gt":
return gt(conditions, data)
case "_in":
return in(conditions, data)
case "_le":
return le(conditions, data)
case "_lt":
return lt(conditions, data)
case "_ne": // -> "_!eq"
return ne(conditions, data)
case "_nin": // -> "_!in"
return nin(conditions, data)
case "_or":
return or(conditions, data)
case "_like":
return like(conditions, data)
case "_nlike": // -> "_!like"
return nlike(conditions, data)
case "_ilike":
return ilike(conditions, data)
case "_nilike": // -> "_!ilike"
return nilike(conditions, data)
case "_not":
return not(conditions, data)
default:
return false, NewErrUnknownOperator(op)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of _nilike: "test"
it could be _not: { _ilike: "test" }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of
_nilike: "test"
it could be_not: { _ilike: "test" }
This can already be done. _nilike
is available for convenience.
e9986f3
to
a0d906a
Compare
bug bash result: ran different queries with |
Relevant issue(s)
Resolves #2367
Description
This PR adds support for case insensitive like and not-like operators. This was requested by a partner.
Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: