10000 Add support for $sortyCount aggregation command by userlerueda · Pull Request #896 · mongomock/mongomock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add support for $sortyCount aggregation command #896

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

userlerueda
Copy link
  • draft for support for $sortByCount in the $facet stage

@userlerueda
Copy link
Author

@mdomke this is my first contribution to the project, I wonder if there is a utility that ignores the order for things such as $sort, in this case the order if relevant for the count, but in the case where count is equal, the order becomes irrelevant, I don't know if there is some helper function that considers this.

Please do let me know, I am happy to add anything that you see necessary help the feature become supported.

@userlerueda
Copy link
Author
userlerueda commented Aug 5, 2024

I have updated and tested but haven't been able to get rid of the assertion error from the diff that states that it is in the incorrect order.

E               AssertionError: Unexpected Diff:
E                       fake.0.pipeline_b.0._id != real.0.pipeline_b.0._id (1 != 2)
E                       fake.0.pipeline_b.1._id != real.0.pipeline_b.1._id (2 != 4)
E                       fake.0.pipeline_b.2._id != real.0.pipeline_b.2._id (4 != 1)

Here is what the return looks like:

[{'_id': 1, 'count': 2}, {'_id': 2, 'count': 2}, {'_id': 4, 'count': 2}, {'_id': 3, 'count': 1}]

Since all of them have a count of 2, technically speaking (or at least up to my knowledge) they could come back from mongodb in any order.

I am not sure if there is a way around this. @mdomke How would you approach it?

@userlerueda userlerueda requested a review from mdomke August 5, 2024 12:32
@mdomke
Copy link
Member
mdomke commented Sep 11, 2024

Can you maybe rebase your changes on the current development version, so that we actually have a test-pipeline where I could look at the error? The tip of the develop-branch uses GitHub Actions.

@userlerueda
Copy link
Author

Sure, let me do that.

@userlerueda userlerueda force-pushed the userlerueda/feat/aggregation-sort-by-count branch from 6ee00c8 to 0ee3720 Compare September 12, 2024 18:54
@userlerueda userlerueda requested a review from mdomke September 12, 2024 18:54
@userlerueda
Copy link
Author
userlerueda commented Sep 12, 2024

@mdomke I just rebased against the latest develop branch. Please let me know if there is anything else I should do.

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

Successfully merging this pull request may close these issues.

2 participants
0