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

Query: Fixes incorrect RequestCharge and missing headers in FeedResponse for ordered cross-partition queries #2357

Merged
merged 9 commits into from
Apr 16, 2021

Conversation

ccurrens
Copy link
Contributor

Pull Request Template

Description

This commit changes the OrderByCrossPartitionQueryPipelineStage to pass along the AdditionalHeaders property from the current enumerator to the page result.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Closing issues

closes #2356

bchong95
bchong95 previously approved these changes Mar 31, 2021
@ealsur
Copy link
Member

ealsur commented Mar 31, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@j82w
Copy link
Contributor

j82w commented Apr 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@j82w
Copy link
Contributor

j82w commented Apr 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@neildsh
Copy link
Contributor

neildsh commented Apr 9, 2021

I would also recommend adding a test to Microsoft.Azure.Cosmos\tests\Microsoft.Azure.Cosmos.EmulatorTests\Query\OrderByQueryTests.cs

that verifies request charges. You can try with:

  • queries that match no documents
  • queries that match documents but return no results because of OFFSET LIMIT
  • queries that match some documents

@ccurrens ccurrens marked this pull request as draft April 13, 2021 15:52
@ccurrens
Copy link
Contributor Author

Let me know if the test added to OrderByQueryTests needs to be adjusted.

@ccurrens ccurrens marked this pull request as ready for review April 13, 2021 19:28
@ccurrens ccurrens requested a review from leminh98 as a code owner April 13, 2021 19:28
@ealsur
Copy link
Member

ealsur commented Apr 13, 2021

/azp run

neildsh
neildsh previously approved these changes Apr 14, 2021
@j82w
Copy link
Contributor

j82w commented Apr 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ccurrens
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 2357 in repo Azure/azure-cosmos-dotnet-v3

@neildsh
Copy link
Contributor

neildsh commented Apr 15, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@j82w
Copy link
Contributor

j82w commented Apr 15, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

neildsh
neildsh previously approved these changes Apr 15, 2021
@j82w
Copy link
Contributor

j82w commented Apr 16, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@j82w j82w changed the title Query: fixes missing headers in FeedResponse for ordered cross-partition queries Query: Fixes incorrect RequestCharge and missing headers in FeedResponse for ordered cross-partition queries Apr 16, 2021
@j82w j82w merged commit 0dc3fc7 into Azure:master Apr 16, 2021
@ccurrens ccurrens deleted the orderby-crosspartition branch May 26, 2022 15:14
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.

FeedResponse<T>.Headers empty for ordered, cross-partition queries
5 participants