Skip to content

Datastore: support property transforms without specifying client values#1

Merged
dnerdy merged 1 commit intov1.21.0_khanfrom
datastore-transforms-without-client-values
Feb 5, 2026
Merged

Datastore: support property transforms without specifying client values#1
dnerdy merged 1 commit intov1.21.0_khanfrom
datastore-transforms-without-client-values

Conversation

@dnerdy
Copy link

@dnerdy dnerdy commented Feb 3, 2026

Summary:
This PR enables callers to pass in an empty entity when creating a mutation or calling PutWithOptions to indicate that no client values should be written when performing property transforms on the server. Currently passing in an entity (or an empty entity) overwrites (or deletes) the server values before applying the transforms because the PropertyMask on the mutation isn't set (which means the server should update the entity on the server to match all client provided values).

See the comment on setMutationProtoPropertyMaskForTransforms for additional details.

Fixes googleapis#13746, though as mentioned in that issue, it might be better to have a separate API that constructs a properly configured mutation that just applies property transforms. (Or, if the current API is kept, it might be nice to allow passing a nil entity when using transforms.)

Test Plan:
I ran the old and new transform integration tests:

GCLOUD_TESTS_GOLANG_PROJECT_ID=khan-prototype go test -run TestIntegration_Transforms_PutAndTransform
GCLOUD_TESTS_GOLANG_PROJECT_ID=khan-prototype go test -run TestIntegration_Transforms_TransformOnly

Summary:
This PR enables callers to pass in an empty entity when creating a mutation or
calling PutWithOptions to indicate that no client values should be written when
performing property transforms on the serfver. Currently passing in an entity
(or an empty entity) overwrites (or deletes) the server values before applying
the transforms because the PropertyMask on the mutation isn't set (which means
the server should update the entity on the server to match all client provided
values).

See the comment on setMutationProtoPropertyMaskForTransforms for additional
details.

Fixes googleapis#13746, though as
mentioned in that issue, it might be better to have a separate API that
constructs a properly configured mutation that just applies property
transforms. (Or, if the current API is kept, it might be nice to allow passing
a nil entity when using transforms.)

Test Plan:
I ran the old and new transform integration tests:
```
GCLOUD_TESTS_GOLANG_PROJECT_ID=khan-prototype go test -run TestIntegration_Transforms_PutAndTransform
GCLOUD_TESTS_GOLANG_PROJECT_ID=khan-prototype go test -run TestIntegration_Transforms_TransformOnly
```
@dnerdy dnerdy requested a review from csilvers February 3, 2026 23:01
Copy link
Member

@csilvers csilvers left a comment

Choose a reason for hiding this comment

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

I read the commit message here and the issue description at googleapis#13746, and while I'm not very familiar with how transforms work -- except, it seems, "not very well" :-} -- what is happening here seems reasonable to me. Hopefully they'll have a fix upstream soon.

@dnerdy
Copy link
Author

dnerdy commented Feb 5, 2026

I read the commit message here and the issue description at googleapis#13746, and while I'm not very familiar with how transforms work -- except, it seems, "not very well" :-} -- what is happening here seems reasonable to me. Hopefully they'll have a fix upstream soon.

Thanks for the review!

@dnerdy dnerdy merged commit 729aa46 into v1.21.0_khan Feb 5, 2026
2 of 3 checks passed
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