-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Sort pattern aggregations fix postgres #6510
Sort pattern aggregations fix postgres #6510
Conversation
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
Codecov Report
@@ Coverage Diff @@
## master #6510 +/- ##
==========================================
+ Coverage 93.96% 94.01% +0.04%
==========================================
Files 169 169
Lines 11801 11829 +28
==========================================
+ Hits 11089 11121 +32
+ Misses 712 708 -4
Continue to review full report at Codecov.
|
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
@dplewis @davimacedo |
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
@dplewis @davimacedo
|
@cbaker6 That PR seems to be open since the last two days. Please notify me when its merged. |
@cbaker6 Any updates on this? |
@srameshr the main branch has the fix. Let me know if you see the Postgres error in the rest of your tests |
@cbaker6 Yes, I still get this error. I synced my branch with main master branch. |
@srameshr from what I see it says your branch is 14 commits behind the master and I can't see that you ran the test again. |
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
…rameshr/parse-server into sort-pattern-aggregations-fix-postgres
@cbaker6 @dplewis @davimacedo There are some test cases failing which is totally unrelated to this commit. Can someone take a look at that? |
@srameshr can run the test a couple of more times by committing? I would like to see if the same errors you just got are reoccurring |
@cbaker6 |
@srameshr I just restarted the build. I'll keep an eye on it |
Thats a flaky test, nothing to do with this PR. It's really difficult to replicate locally otherwise it would be fixed. |
return query.aggregate(pipeline); | ||
}) | ||
.then(results => { | ||
expect(results.length).toEqual(5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand this expect
to show group by multiple columns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is messed up in my repo. Il create a new PR
Closing via #6522 |
No description provided.