Aggregate supports group by date fields #5538
Conversation
|
Did you figure something out? |
|
The tests are passing in my environment but didn't pass in Travis. I'm trying to understand why. |
| .then(results => { | ||
| const counts = results.map(result => result.count); | ||
| expect(counts.length).toBe(3); | ||
| expect(counts.sort()).toEqual([1, 2, 4]); |
There was a problem hiding this comment.
If group by date works, wouldn't the results be obj1 + obj3 is one group and obj2 is another.
count would be [1, 2] what does the 4 come from?
There was a problem hiding this comment.
4 come from the default data that is added before each test. It returns [ objectId: undefined, count: 4]
There was a problem hiding this comment.
|
Which version of mongo are you running locally? |
|
Antonios-MacBook-Pro-2:parse-server davimacedo$ mongod --version |
|
I think in this new version I'm running mongo works for undefined values, but it doesn't for version 4.0.4. I just added the match stage to ensure that the undefined dates (the ones added before each test) are not processed. Let's see if it will pass in Travis now. |
|
It passed. Conclusion:
In both scenario you can add a match stage to filter out the data before the groupBy. |
| @@ -379,18 +379,23 @@ describe('Parse.Query Aggregate testing', () => { | |||
| }); | |||
|
|
|||
| it_exclude_dbs(['postgres'])( | |||
There was a problem hiding this comment.
This should work on postgres right?
There was a problem hiding this comment.
I am not sure. I will check.
There was a problem hiding this comment.
The group by date field works, but the test fails because the $exists is not implemented for match stage on postgres. I included an additional test only for postgres.
dplewis
changed the title
|
@davimacedo Thanks! |
* it actually supports group by date fields * Changing the field name again to see Travis logs * Adding match stage to the test * Adding test for group by date fields on postgres
This PR aims to affirm that Parse Server currently DOES support group by date fields.
I started working on this because I've just setup a brand new environment for contributing to Parse Server and the test below keeps failing to me when I run coverage npm script on master:
https://github.com/parse-community/parse-server/blob/master/spec/ParseQuery.Aggregate.spec.js#L381
I found out that the group by date fields actually works for normal cases and only does not work when you have dirty data, meaning a non date value in a field that is expected to be a date. I was only able to simulate this problem by doing:
After this, I've just dropped the database again and run the tests. Then the group by was working again.
Probably this test is passing in Travis and maintainers' environments because they have legacy data in their databases. According to my tests, if any of them drop the current test database and run the tests again, this test will fail.
In summary, what I did in this PR:
https://github.com/parse-community/parse-server/blob/master/src/Adapters/Storage/Mongo/MongoStorageAdapter.js#L768
@dplewis do you mind to take a look on this and check if you agree?