-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: use proper indexes for full text search #4787
Conversation
One other thing that this PR changes is that it uses like in favour of ilike, which means that attribute queries for contains and like won't be case independent now. I am not sure if we should remove it for this PR. cc @srikanthccv |
Yes, that should have a separate issue for itself. |
Have made the changes, now it will only apply for body . Attributes and resource attributes won't be touched. |
@@ -72,6 +73,7 @@ func getPath(keyArr []string) string { | |||
|
|||
func getJSONFilterKey(key v3.AttributeKey, op v3.FilterOperator, isArray bool) (string, error) { | |||
keyArr := strings.Split(key.Key, ".") | |||
// i.e it should be at least body.name, and not something like body | |||
if len(keyArr) < 2 { |
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.
does this account for body.
where there is just space after the .
?
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.
keys dont contain space, it will be a invalid query
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 will be returned before reaching this point ?
Fixes #4259
changes
attributes search with operatorcontains
will also uselower(attribute)
now but there is no change for them.