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

PatchOperation.Set cannot handle null values #2931

Closed
loraderon opened this issue Dec 14, 2021 · 15 comments · Fixed by #3093
Closed

PatchOperation.Set cannot handle null values #2931

loraderon opened this issue Dec 14, 2021 · 15 comments · Fixed by #3093
Assignees
Labels
blocked feature-request New feature or request needs-investigation patch No contract changes. (Bug fixes, doc bugs, etc.)
Milestone

Comments

@loraderon
Copy link

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug
PatchOperation.Set throws ArgumentNullException when value is null.

To Reproduce
await container.PatchItemAsync(id, PartitionKey.None, new List { PatchOperation.Set("/name", null) });

Expected behavior
Value should be set to null.

Actual behavior

System.ArgumentNullException: Value cannot be null. (Parameter 'value')
at Microsoft.Azure.Cosmos.PatchOperationCore`1..ctor(PatchOperationType operationType, String path, T value)
at Microsoft.Azure.Cosmos.PatchOperation.Set[T](String path, T value)

Environment summary
SDK Version: 3.23.0
OS Version (e.g. Windows, Linux, MacOSX) Windows 11

Additional context
According to @sajeetharan (https://stackoverflow.com/a/70194090/22092), this seems to be a known issue but I couldn't find any matching issue in github.
image

@j82w j82w added Batch Batch related issue needs-investigation patch No contract changes. (Bug fixes, doc bugs, etc.) feature-request New feature or request and removed Batch Batch related issue labels Dec 14, 2021
@yang-er
Copy link

yang-er commented Jan 16, 2022

Hi, I have a workaround as below which could resolve your problem:

await container.PatchItemAsync(id, PartitionKey.None, new List { PatchOperation.Set("/name", new MemoryStream(Encoding.UTF8.GetBytes("null"))) });

My thought is that here we shouldn't add null checks for value.

@j82w j82w added this to the Triage milestone Jan 24, 2022
@Jezternz
Copy link

Jezternz commented Feb 4, 2022

This sounds like it should work, but it appears that to make this workaround work as expected, modifications to json serialization would somehow have to happen:
image

@ghost
Copy link

ghost commented Feb 4, 2022

This works for me.

Instead of using null I just create a JProperty with the value null. And use that value.

private JToken NullValue { get { return new JProperty("nullValue", null).Value; } }
And use it like this
operations.Add(PatchOperation.Set("\customer", value ?? NullValue));

@Jezternz
Copy link

Jezternz commented Feb 6, 2022

Edit: Thanks @rvdvelden, that works perfectly!

@yang-er
Copy link

yang-er commented Feb 6, 2022

BTW, a much easier way to get a null JToken: JValue.CreateNull()

@kirankumarkolli
Copy link
Member

@deparash what's the expected server contract for such scenarios?

@kirankumarkolli
Copy link
Member

Update: (offline closure with @deparash )

Value is expected and it should be JSON NULL.
Current implementation is using JSON.NET for writing and it can leverage WriteNull() instead.

@kirankumarkolli
Copy link
Member

@SchintaMicrosoft assigned it to you.

@onionhammer
Copy link
Contributor

This should still be open, not closed. There are hacky workarounds for now, commented above

@j82w
Copy link
Contributor

j82w commented May 5, 2022

Github automatically closes the issue when the PR is merged. A new version should be released hopefully by tomorrow with the fix.

@onionhammer
Copy link
Contributor

Not fixed in 3.26.2

@j82w
Copy link
Contributor

j82w commented May 5, 2022

Sorry about the confusion. The fix is going out in 3.27.0 which can be tracked here: #3172
The 3.27 release is waiting on this fix: #3168
3.26.2 is a hotfix to unblock a customer hitting an issue in production which is why it only has 3 required changes cherry picked.

@j82w
Copy link
Contributor

j82w commented May 6, 2022

@onionhammer please try the latest SDK release 3.27.0 https://github.com/Azure/azure-cosmos-dotnet-v3/releases/tag/3.27.0

@onionhammer
Copy link
Contributor

Yep, thanks, that fixes it

@gagwithgaffer
Copy link

Using Microsoft.Azure.Cosmos 3.28 and i dont see any fix? will have to resort to the above said workaround from rvdvelden

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked feature-request New feature or request needs-investigation patch No contract changes. (Bug fixes, doc bugs, etc.)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants