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

Fix for countDocument regression #1711

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix for countDocument regression #1711

wants to merge 2 commits into from

Conversation

maheshrajamani
Copy link
Contributor

@maheshrajamani maheshrajamani commented Nov 13, 2024

What this PR does:
Count command fix

Which issue(s) this PR fixes:
Fixes #1708

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@maheshrajamani maheshrajamani requested a review from a team as a code owner November 13, 2024 22:48
})
// Documents read until pageState available, max records read is deleteLimit + 1 TODO
// COMMENTS
.whilst(AsyncPagingIterable::hasMorePages)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question: where is the "max count" checked? Looks like this would read through all entries and not stop at specific count? (I am probably missing something)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be set in the CQL limit, the page size we tell the driver is how to chunk LIMIT number of rows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like the code we came up with for in memory sorting for tables

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Yes, since that's within SimpleStatement being passed.

@amorton
Copy link
Contributor

amorton commented Nov 14, 2024

moving to draft due to last minute situation, the simple fix is #1715

@amorton amorton marked this pull request as draft November 14, 2024 01:22
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.

(regression) countDocuments returns a count capped at 20 for collections
3 participants