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

BED-4726 added new ~eq predicate for filtering on strings #808

Merged
merged 12 commits into from
Aug 29, 2024

Conversation

mvlipka
Copy link
Contributor

@mvlipka mvlipka commented Aug 22, 2024

Description

This change adds the ability to query text columns in our database, if specified to be allowed, for approximately equals.

Motivation and Context

This was created as part of the saved queries improvements as we required the ability to search for saved queries by their description. Previously it was only possible to search text columns for equals and not equals, this would be frustrating as you may only remember a portion of the description that you wrote.
We also added support for text searching the saved_queries.name column.

The gin index was added to the relevant columns in order to improve text search performance.
https://www.postgresql.org/docs/9.1/textsearch-indexes.html

This PR addresses: BED-4726

How Has This Been Tested?

I created a couple of sample rows in the saved_queries table
image
NOTE: all of the queries for this must belong to the user that is performing the API request

I then sent a query to search for a description
image
We can see here that only one of the descriptions appears

Querying for a description that does not exist:
image

Querying for a name that returns multiple results:
image

Querying for a name returns a single result
image

Querying for a description with spaces in it:
image

Querying for a name with spaces that contains the term test in multiple rows, but only returns the case-insensitive match
image

Testing querying with %20 to represent a space instead of +
image

Testing the existing eq predicate works with spaces
image

Testing the existing neq predicate works as it did previously
image

Screenshots (optional):

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Database Migrations

Checklist:

@mvlipka mvlipka added documentation Improvements or additions to documentation enhancement New feature or request api A pull request containing changes affecting the API code. labels Aug 22, 2024

GreaterThanSymbol string = ">"
GreaterThanOrEqualsSymbol string = ">="
LessThanSymbol string = "<"
LessThanOrEqualsSymbol string = "<="
EqualsSymbol string = "="
NotEqualsSymbol string = "<>"
ApproximatelyEqualSymbol string = "ILIKE"
Copy link
Contributor Author

@mvlipka mvlipka Aug 22, 2024

Choose a reason for hiding this comment

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

ILIKE allows for case insensitive search

Copy link
Contributor

Choose a reason for hiding this comment

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

ILIKE it!

@mvlipka mvlipka marked this pull request as ready for review August 27, 2024 18:11
@mvlipka mvlipka changed the title DRAFT: BED-4726 added new ~eq predicate for filtering on strings BED-4726 added new ~eq predicate for filtering on strings Aug 27, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be v5.16.0 correct? Or is the goal to have this in the upcoming release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably cutting it close to the next release since we're about to cut the RC.
I'll throw it in for v5.16.0

@@ -13929,7 +13929,7 @@
},
"api.params.predicate.filter.string": {
"type": "string",
"description": "Filter results by column string value. Valid filter predicates are `eq`, `neq`.\n"
"description": "Filter results by column string value. Valid filter predicates are `eq`, `neq`, `~eq`.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit but should ~eq go before ne 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see that, it's related to eq so would be nice to have it read like a cascading
"equals, approximately equals, not equals"

cmd/api/src/model/saved_queries.go Show resolved Hide resolved
cmd/api/src/model/filter.go Show resolved Hide resolved

GreaterThanSymbol string = ">"
GreaterThanOrEqualsSymbol string = ">="
LessThanSymbol string = "<"
LessThanOrEqualsSymbol string = "<="
EqualsSymbol string = "="
NotEqualsSymbol string = "<>"
ApproximatelyEqualSymbol string = "ILIKE"
Copy link
Contributor

Choose a reason for hiding this comment

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

ILIKE it!

Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Thanks for your patience and adding the extra test coverage! 🚀

@mvlipka mvlipka merged commit 8126ba1 into main Aug 29, 2024
4 checks passed
@mvlipka mvlipka deleted the BED-4726-approximately-equals-predicate branch August 29, 2024 19:36
@github-actions github-actions bot locked and limited conversation to collaborators Aug 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api A pull request containing changes affecting the API code. documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants