[Internal] Query: Adds support for binary encoded ReadFeed and Read responses #4516
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Maya-Painter
changed the title
Maya-Painter
changed the title
Maya-Painter
requested review from
khdang,
sboshra,
adityasa,
neildsh,
kirankumarkolli,
ealsur,
FabianMeiswinkel and
kirillg
as code owners
| @@ -241,7 +241,7 @@ internal override void PopulateRequestOptions(RequestMessage request) | |||
| // All query APIs (GetItemQueryIterator, GetItemLinqQueryable and GetItemQueryStreamIterator) turn into ReadFeed operation if query text is null. | |||
| // In such a case, query pipelines are still involved (including QueryRequestOptions). In general backend only honors SupportedSerializationFormats | |||
| // for OperationType Query but has a bug where it returns a binary response for ReadFeed API when partition key is also specified in the request. | |||
| if (request.OperationType == OperationType.Query) | |||
| if (request.OperationType == OperationType.Query || request.OperationType == OperationType.ReadFeed) | |||
There was a problem hiding this comment.
This would require a few test updates. Kindly confirm (otherwise I am guessing we would have test gaps).
There was a problem hiding this comment.
It is passing all gates. Which tests are you thinking about in particular? Before the rewrite there were multiple readfeed failures (same as we saw when the emulator was first upgraded).
Maya-Painter
changed the title
| @@ -293,6 +294,11 @@ internal class RequestInvokerHandler : RequestHandler | |||
| { | |||
| request.Headers.ContentType = RuntimeConstants.MediaTypes.JsonPatch; | |||
| } | |||
| else if (operationType == OperationType.Read) | |||
| { | |||
| request.Headers.CosmosMessageHeaders.SupportedSerializationFormats = DocumentQueryExecutionContextBase.DefaultSupportedSerializationFormats; | |||
There was a problem hiding this comment.
Apparently SupportedSerializationFormats header isn't honored by backend for read operations, even though it is expected to. Using ContentSerializationFormat for testing for now.
There was a problem hiding this comment.
@Maya-Painter - Shivam merged a PR for this a while back (several weeks) - did you test this against a prod account or emulator - and if emulator with /EnablePreview or without?
There was a problem hiding this comment.
Tested on both a prod account and the newest emulator with /EnablePreview and got the same result. I've looked at Shivam's change and I see no reason why it shouldn't work, so this is surprising. I assume the change should have been picked up in the newest emulator? @FabianMeiswinkel
Description
This PR adds support for binary encoded ReadFeed responses.
Type of change
Please delete options that are not relevant.