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

array_type_field_filter: Adds contains filter for postgresDB #1865

Closed

Conversation

shubhisood
Copy link

Adds contains filter to make searches on Array type field.

Fixes #342 of loopback-postgres connector

Checklist

  • Sign off your commits with DCO (Developer Certificate of Origin)
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Commit messages are following our guidelines

@raymondfeng
Copy link
Contributor

@shubhisood Thank you for the PR. Please sign DCO (https://loopback.io/doc/en/contrib/code-contrib.html).

@@ -88,6 +88,7 @@ const operators = {
ilike: 'ILIKE',
nilike: 'NOT ILIKE',
regexp: 'REGEXP',
contains: '@>',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that the operator needs to be supported by other LoopBack db connectors beyond Postgres. Do we have some ideas about the feasibility of other DBs?

Copy link

Choose a reason for hiding this comment

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

@raymondfeng This feature is so much needed. We literally had to hack our way into Loopback as we needed it extensively. Please figure out a way to get this merged.

Copy link
Author

Choose a reason for hiding this comment

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

@raymondfeng currently i have added it only for postgres db, not sure how we may implement it for other db connectors as well, i may need to look into this..

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @raymondfeng that we must keep other connectors in mind when making any changes in juggler.

Is it possible to implement array coercion of contains filter inside the postgresql connector?

In MongoDB, we have the concept of extended operators, see line 507 later in this file.

https://github.com/strongloop/loopback-datasource-juggler/blob/d953eabe30d793642b9268014efc5f4da4633e29/lib/model-utils.js#L507

Can we perhaps use the same mechanism for PostgreSQL?

Copy link
Author

Choose a reason for hiding this comment

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

I need to check into this @bajtos will inform you after analysis.

Copy link
Author

Choose a reason for hiding this comment

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

@bajtos you are right after using allowExtendedOperators option within postgres connector model, I am able to handle the case for array field filter, we can close this PR after postgres connector specfic PR gets approved.

@shubhisood shubhisood changed the title array_type_field_filter: Fixes search based on array type fields in postgres DB array_type_field_filter: Adds contains filter for postgresDB Sep 5, 2020
1. Adds new filter for array type field.
2. From this filter you can search from array type field.

Signed-off-by: shubhisood <shubhi.sood@sourcefuse.com>
@shubhisood
Copy link
Author

Travis build is failing because the following PR related to juggler needs to be merged first:
#1865

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

Thank you @shubhisood for the pull request. It makes a lot of sense to me to support additional database-specific operators like contains 👍

However, we need to find a solution that will work for all juggler users, regardless of the connector they are using.

@@ -88,6 +88,7 @@ const operators = {
ilike: 'ILIKE',
nilike: 'NOT ILIKE',
regexp: 'REGEXP',
contains: '@>',
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @raymondfeng that we must keep other connectors in mind when making any changes in juggler.

Is it possible to implement array coercion of contains filter inside the postgresql connector?

In MongoDB, we have the concept of extended operators, see line 507 later in this file.

https://github.com/strongloop/loopback-datasource-juggler/blob/d953eabe30d793642b9268014efc5f4da4633e29/lib/model-utils.js#L507

Can we perhaps use the same mechanism for PostgreSQL?

@bajtos
Copy link
Member

bajtos commented Sep 15, 2020

Closing per #1865 (comment)

after using allowExtendedOperators option within postgres connector model, I am able to handle the case for array field filter, we can close this PR after postgres connector specfic PR gets approved.

@bajtos bajtos closed this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Patches contributed by community feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter by array of values
4 participants