-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Cosmos] Honor max_item_count for query_items(feed_range=...) #46469
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
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3202,6 +3202,60 @@ def __GetBodiesFromQueryResult(result: dict[str, Any]) -> list[dict[str, Any]]: | |
| response_hook(self.last_response_headers, partial_result) | ||
| # if the prefix partition query has results lets return it | ||
| if results: | ||
| # Honor the user-requested page size (maxItemCount) across the K overlapping | ||
| # inner physical PK ranges. Each inner __Post above honors max_item_count per | ||
| # range, so the merged result can hold up to K * max_item_count documents. | ||
| # Truncate to the user-requested cap so a single page never exceeds it. | ||
| # | ||
| # NOTE: Use a truthy check (mirrors the contract used by _base.GetHeaders, | ||
| # which only emits the x-ms-max-item-count header when options['maxItemCount'] | ||
| # is truthy). max_item_count=0/None/missing all mean "use the server default | ||
| # page size" and must be a no-op here, otherwise we would silently return | ||
| # empty pages while the server-side default page actually returned data. | ||
| # | ||
| # Known limitation (intentionally deferred — tracked separately as a | ||
| # follow-up: "[Cosmos] feed_range query continuation token replays documents | ||
| # from non-cursor PK ranges"): | ||
| # The continuation token surfaced to the caller is only the K-th inner | ||
| # range's x-ms-continuation. When the caller round-trips that token on the | ||
| # next page, _CosmosClientConnection__QueryFeed re-resolves the K overlapping | ||
| # ranges and sends the same caller-supplied continuation to every inner POST. | ||
| # Range K-1's continuation against ranges 0..K-2 is undefined server-side | ||
| # (may error, may restart from the beginning, may return undefined slices), | ||
| # so callers paginating a feed_range that overlaps multiple PK ranges may | ||
| # observe duplicates, missing documents, or non-terminating iteration. The | ||
| # correct fix is a composite continuation token spanning all K inner PK | ||
| # ranges. Until that lands, this branch only delivers a correct *first* page | ||
| # for multi-range feed_range queries. | ||
| # | ||
| # Mitigation for the deferred limitation: when truncation actually | ||
| # discards documents, the surfaced continuation token would describe | ||
| # the wrong cursor (it is the last inner range's token, but documents | ||
| # from earlier ranges and from the truncated tail were dropped). To | ||
| # avoid silent data loss, we strip the continuation header below when | ||
| # truncation occurs, so the truncated page is observed as terminal | ||
| # rather than producing wrong results on resume. | ||
|
Comment on lines
+3205
to
+3237
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. waaaay too long of a comment - we probably only need the first paragraph and the last one at most |
||
| max_item_count = options.get("maxItemCount") | ||
| docs = results.get("Documents") | ||
| if max_item_count and isinstance(docs, list): | ||
| try: | ||
| cap = int(max_item_count) | ||
| except (TypeError, ValueError): | ||
| cap = 0 | ||
| if 0 < cap < len(docs): | ||
| results["Documents"] = docs[:cap] | ||
| # Keep the internal _count field consistent with the truncated | ||
| # Documents list so any downstream consumer that introspects | ||
| # the merged dict observes a coherent shape. | ||
| results["_count"] = cap | ||
|
tvaron3 marked this conversation as resolved.
|
||
| # The merged page was assembled from multiple inner PK ranges and | ||
| # then truncated. The continuation token from the last inner range | ||
| # only describes progress for that range, not for the truncated | ||
| # tail or for ranges past the cursor, so resuming with it would | ||
| # silently skip documents. Until a composite continuation token is | ||
| # implemented, suppress the misleading token so callers see the | ||
| # truncated page as terminal rather than experiencing data loss. | ||
| self.last_response_headers.pop(http_constants.HttpHeaders.Continuation, None) | ||
| if self.last_response_headers.get(http_constants.HttpHeaders.IndexUtilization) is not None: | ||
| index_metrics_raw = self.last_response_headers[http_constants.HttpHeaders.IndexUtilization] | ||
| self.last_response_headers[http_constants.HttpHeaders.IndexUtilization] = ( | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.