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

Added support for the contains query operator #11

Closed
wants to merge 2 commits into from
Closed

Added support for the contains query operator #11

wants to merge 2 commits into from

Conversation

recipher
Copy link

@recipher recipher commented Sep 7, 2015

This should fix #10.

I've made the query case-insensitive. I guess it would be better for this to be an option, but I couldn't see a reasonable way to do that.

Thanks.

@jmdobry
Copy link
Member

jmdobry commented Sep 7, 2015

The "contains" operator needs to work for arrays as well (see how it's implemented in js-data.js ). Also, can you remove your changes to dist/js-data-rethinkdb.js from the PR?

@recipher
Copy link
Author

recipher commented Sep 7, 2015

Just to be clear, when you say it should work with arrays, do you mean

  1. Check an array property to see if any of the elements in the array contains the string term?
  2. Check to see if a string property contains any of the elements in an array term?

I'll need to investigate ReQL to check to see if this is possible. The contains operator in ReQL needs to work in conjunction with the match operator, it seems to me.

http://rethinkdb.com/api/javascript/contains/

I'll close this and create another, without the changes to dist.

@recipher
Copy link
Author

recipher commented Sep 7, 2015

Looking at http://www.js-data.io/docs/query-syntax

It seems like option 1 is the requirement.

// Get all users where user.roles contains "admin"
User.filter({
  where: {
    roles: {
      'contains': 'admin'
    }
  }
});

Is there a simple way to tell whether roles (in the example above), is an array property or a string property, since I'll use ReQL operator match for strings and operator contains for arrays.

@jmdobry
Copy link
Member

jmdobry commented Sep 7, 2015

I'm not for sure on how to implement it, which is why I didn't implement it in the first place.

@recipher
Copy link
Author

recipher commented Sep 7, 2015

Ah, ok. It's not worth implementing it for at least the first case (string matching) then? And then add the second case later?

Under normal circumstances, I'd expect these to be 2 different operations (as they are in MongoDB and RethinkDB). In any NoSQL DB, since a column can have any "type", it might prove problematic to determine which type a column is.

Might it make more sense (and more work) to split them into 2 separate ops:

// Get all users where user.roles contains "admin"
User.filter({
  where: {
    roles: {
      'contains': 'admin'
    }
  }
});

And

// Get all users where user.name is like "john"
User.filter({
  where: {
    name: {
      'like': 'john'
    }
  }
});

@jmdobry
Copy link
Member

jmdobry commented Sep 7, 2015

I'll have to think about it some more. js-data-sql actually has a "like" operator. I want consistency across the store and the adapters.

@recipher
Copy link
Author

recipher commented Sep 7, 2015

Grand. Thanks.


Johnny H
07971 880871

On Mon, Sep 7, 2015 at 5:29 PM, Jason Dobry [email protected]
wrote:

I'll have to think about it some more. js-data-sql actually has a "like" operator. I want consistency across the store and the adapters.

Reply to this email directly or view it on GitHub:
#11 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It does not support 'contains' keyword in findAll() query
2 participants