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

Doesn't act as expected ! #117

Closed
codelovesme opened this issue Mar 20, 2017 · 6 comments
Closed

Doesn't act as expected ! #117

codelovesme opened this issue Mar 20, 2017 · 6 comments

Comments

@codelovesme
Copy link

codelovesme commented Mar 20, 2017

Hi,
I was just testing the library and I faced that in one exact case it doesn't return expected result.
test1

//given
                        let obj1 = {
                            c: { d: "d" }
                        };
                        let obj2 = {
                            a: "done",
                            b: 34,
                            c: { d: "d", e:"e" }
                        };
 //when
                        let result = sift(obj1, [obj2]); 
// result = [obj2]

test2

//given
//collection contains obj2
//when
                       let result = collection.find(obj1); 
// result = [ ]

Shouldn't the results of both tests be the same ?
Mongodb returns obj2 in that case :

                       let obj1 = {
                            "c.d": "d"
                        };
                       let result = collection.find(obj1); 
// result = [obj2]
@crcn
Copy link
Owner

crcn commented Apr 11, 2017

If I'm understanding correctly, you'd expect this in sift:

const obj1 = { c: { d: 'd' }};
const obj2 = { a: 'done', c: { d: 'd' }};

const result = sift(obj1, [obj2]); // []

Where result would be an empty array. Currently sift returns obj2 based on the obj1 query -- this is intented behavior from when I first added the functionality a few years ago. I'd be open to changing this however to reflect mongodb's behavior a bit.

Mongodb returns obj2 in that case

Sift supports string queries like this: https://github.com/crcn/sift.js/blob/master/test/operations-test.js#L148

@PascalPflaum
Copy link

PascalPflaum commented Oct 24, 2017

That the two queries
collection.find(c: { d: 'd'});
and
collection.find('c.d': 'd');

have different results is by purpose in MongoDB and one of the tiny details, that makes this query language so powerful.

  • Query 1 looks for a property c that is equal to { d: "d" }
  • Query 2 looks for a nested property d in c that is equal to d

I'm in favor of reflecting the mongoDB behaviour as close as possible, something that looks like MongoDB Query Language, but doesn't behave like MongoDB Query Language is worse than a total different system in my opinion.

@crcn
Copy link
Owner

crcn commented Oct 24, 2017

something that looks like MongoDB Query Language, but doesn't behave like MongoDB Query Language is worse than a total different system in my opinion.

Agreed. I'll move forward with this change then. :)

@crcn crcn added the feature label Oct 24, 2017
@codelovesme
Copy link
Author

Thank you for considering this as a next step.
I will be waiting the results.

@codelovesme
Copy link
Author

Hi @crcn
I couldn't follow.
Is there any progress on this issue ?

@crcn
Copy link
Owner

crcn commented Dec 22, 2018

Yep, sorry for not getting to this sooner, this ticket is long overdue. 😅

Just added the functionality to 8.0.0, you can find the code here: https://github.com/crcn/sift.js/blob/master/src/index.js#L470. Also published to NPM under beta, but I'd hold off on using 8.0.0 in production until I add more tests.

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

No branches or pull requests

3 participants