Skip to content

test: add a test for JSON materialization#780

Merged
olavloite merged 3 commits into
mainfrom
json-stream-test-and-comment
Jun 30, 2026
Merged

test: add a test for JSON materialization#780
olavloite merged 3 commits into
mainfrom
json-stream-test-and-comment

Conversation

@olavloite

Copy link
Copy Markdown
Collaborator

During an attempt at optimizing the way that JSON queries are materialized, we discovered that Entity Framework does not dispose the stream that it receives. This means that we cannot easily optimize this by using for example a pooled buffer.

During an attempt at optimizing the way that JSON queries are materialized, we discovered
that Entity Framework does not dispose the stream that it receives. This means that we
cannot easily optimize this by using for example a pooled buffer.
@olavloite olavloite requested review from a team as code owners June 29, 2026 12:44
@olavloite olavloite requested a review from sakthivelmanii June 29, 2026 12:44

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds a unit test to verify that JSON queries materialize correctly and refactors CreateUtf8Stream in SpannerStructuralJsonTypeMapping to document the memory stream lifecycle and explain why pooled resources are avoided. The feedback suggests using an asynchronous await using var statement in the new test to reduce nesting and maintain consistency with the rest of the test suite.

Comment thread Google.Cloud.EntityFrameworkCore.Spanner.Tests/EntityFrameworkMockServerTests.cs Outdated
@olavloite

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new unit test JsonQuery_Materializes_Correctly to verify that JSON queries materialize correctly into structured objects. Additionally, it refactors the CreateUtf8Stream method in SpannerStructuralJsonTypeMapping to use a block body and adds an explanatory comment detailing why MemoryStream is allocated directly rather than using pooled resources to avoid memory leaks. There are no review comments, so I have no feedback to provide.

@olavloite olavloite merged commit ad9883b into main Jun 30, 2026
24 checks passed
@olavloite olavloite deleted the json-stream-test-and-comment branch June 30, 2026 14:24
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.

2 participants