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

fix(spanner): json null handling #10660

Merged
merged 5 commits into from
Aug 20, 2024
Merged

fix(spanner): json null handling #10660

merged 5 commits into from
Aug 20, 2024

Conversation

egonelbre
Copy link
Contributor

Improves performance and fixes a potential corner case with regards to mutations.

By using a string comparison the compiler can significantly optimize
the check.
The caller can change the result from these calls and this may
cause weird bugs down the line. Instead allocate a new slice.
@egonelbre egonelbre requested review from a team as code owners August 9, 2024 08:52
@egonelbre egonelbre changed the title Improvements to json null handling fix(spanner): json null handling Aug 9, 2024
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Aug 10, 2024
@rahul2393
Copy link
Contributor

Hello @egonelbre Can you please share what was the corner case with regards to mutation?

@egonelbre
Copy link
Contributor Author

@rahul2393 I added a test to show the main idea. Basically, when a user changes MarshalJSON result, they might accidentally change the underlying jsonNullBytes content, causing all following calls to return the wrong data.

@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 19, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 19, 2024
@egonelbre
Copy link
Contributor Author

@rahul2393 in case it's not clear, I'm not able to merge any of my PR-s myself.

@rahul2393 rahul2393 enabled auto-merge (squash) August 20, 2024 12:00
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 20, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 20, 2024
@rahul2393 rahul2393 merged commit 4c519e3 into googleapis:main Aug 20, 2024
8 checks passed
@egonelbre egonelbre deleted the null branch August 20, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants