Skip to content

Commit

Permalink
Query: Fixes "System.ArgumentException: Stream was not readable." whe…
Browse files Browse the repository at this point in the history
…n using WithParameterStream (#3155)

The query definition holds the memory stream passed in by the user (this was introduced to support query on encrypted properties). That stream is then read and disposed to send the query over the wire. The issue is the query plan will be serialized multiple times for cases like cross partition queries and for scenarios where the query plan cannot be generated locally. Reserializing the query plan seems to be by design because the query plan can rewrite the query.

The fix reads the stream upfront to a string which is stored and later passed.
  • Loading branch information
kr-santosh authored Apr 29, 2022
1 parent eec5de5 commit 9411d26
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
// if the SqlParameter has stream value we dont pass it through the custom serializer.
if (sqlParameter.Value is SerializedParameterValue serializedEncryptedData)
{
using (StreamReader streamReader = new StreamReader(serializedEncryptedData.valueStream))
{
string parameterValue = streamReader.ReadToEnd();
writer.WriteRawValue(parameterValue);
}
writer.WriteRawValue(serializedEncryptedData.rawSerializedJsonValue);
}
else
{
Expand Down
10 changes: 8 additions & 2 deletions Microsoft.Azure.Cosmos/src/Query/v3Query/QueryDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,16 @@ public QueryDefinition WithParameter(string name, object value)
/// <returns>An instance of <see cref="QueryDefinition"/>.</returns>
public QueryDefinition WithParameterStream(string name, Stream valueStream)
{
string rawSerializedJsonValue = null;
using (StreamReader streamReader = new StreamReader(valueStream))
{
rawSerializedJsonValue = streamReader.ReadToEnd();
}

// pack it into an internal type for identification.
SerializedParameterValue serializedParameterValue = new SerializedParameterValue
{
valueStream = valueStream
rawSerializedJsonValue = rawSerializedJsonValue
};

return this.WithParameter(name, serializedParameterValue);
Expand Down Expand Up @@ -200,6 +206,6 @@ IEnumerator IEnumerable.GetEnumerator()

internal struct SerializedParameterValue
{
internal Stream valueStream;
internal string rawSerializedJsonValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,36 @@ public async Task QueryStreamValueTest()
Assert.AreEqual(pageCount, fromStreamCount);
}

// get result across pages,multiple requests by setting MaxItemCount to 1.
foreach (QueryDefinition queryDefinition in queryDefinitions)
{
toStreamCount = 0;
fromStreamCount = 0;

List<dynamic> allItems = new List<dynamic>();
int pageCount = 0;
using (FeedIterator<dynamic> feedIterator = containerSerializer.GetItemQueryIterator<dynamic>(
queryDefinition: queryDefinition,
requestOptions: new QueryRequestOptions { MaxItemCount = 1 }))
{
while (feedIterator.HasMoreResults)
{
// Only need once to verify correct serialization of the query definition
FeedResponse<dynamic> response = await feedIterator.ReadNextAsync(this.cancellationToken);
Assert.AreEqual(response.Count, response.Count());
allItems.AddRange(response);
pageCount++;
}
}

Assert.AreEqual(2, allItems.Count, $"missing query results. Only found: {allItems.Count} items for query:{queryDefinition.ToSqlQuerySpec().QueryText}");

// There should be no call to custom serializer since the parameter values are already serialized.
Assert.AreEqual(0, toStreamCount, $"missing to stream call. Expected: 0 , Actual: {toStreamCount} for query:{queryDefinition.ToSqlQuerySpec().QueryText}");
Assert.AreEqual(pageCount, fromStreamCount);
}


// Standard Cosmos Serializer Used

CosmosClient clientStandardSerializer = TestCommon.CreateCosmosClient(useCustomSeralizer: false);
Expand Down

0 comments on commit 9411d26

Please sign in to comment.