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

Handle the zero limit case explicitly for MongoDB aggregation #187

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

suresh-prakash
Copy link
Contributor

@suresh-prakash suresh-prakash commented Dec 12, 2023

MongoDB find() implementation handles the 0-limit case and returns no documents.
Whereas the deprecation alternative method aggregate() implementation throws an exception on a 0-limit for MongoDB, while it works as expected for Postgres.

This PR aims at fixing that bug by manually handling the 0-limit case (as there is no way to handle this in MongoDB aggregation pipeline) and adds an integration test to cover that scenario.

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (17c84c4) 80.71% compared to head (b1d5d5b) 80.60%.

Files Patch % Lines
.../documentstore/mongo/query/MongoQueryExecutor.java 61.53% 5 Missing ⚠️
...ypertrace/core/documentstore/query/Pagination.java 0.00% 0 Missing and 2 partials ⚠️
...cumentstore/mongo/query/MongoPaginationHelper.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #187      +/-   ##
============================================
- Coverage     80.71%   80.60%   -0.12%     
- Complexity      931      932       +1     
============================================
  Files           149      149              
  Lines          4258     4275      +17     
  Branches        354      356       +2     
============================================
+ Hits           3437     3446       +9     
- Misses          585      590       +5     
- Partials        236      239       +3     
Flag Coverage Δ
integration 80.60% <57.89%> (-0.12%) ⬇️
unit 59.64% <26.31%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Test Results

  37 files  ±0    37 suites  ±0   31s ⏱️ +2s
227 tests ±0  227 ✔️ ±0  0 💤 ±0  0 ±0 
469 runs  +2  469 ✔️ +2  0 💤 ±0  0 ±0 

Results for commit b1d5d5b. ± Comparison against base commit 17c84c4.

@suresh-prakash suresh-prakash merged commit 35d87df into main Dec 12, 2023
5 of 7 checks passed
@suresh-prakash suresh-prakash deleted the handle_zero_limit_case_for_mongo_aggregation branch December 12, 2023 09:28
@sarthak77
Copy link
Member

MongoDB find() implementation handles the 0-limit case and returns no documents.

Does it return no documents or all 🤔

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.

3 participants