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

Datastore fails when saving entity with null attribute value #1022

Closed
toboid opened this issue Dec 14, 2015 · 11 comments
Closed

Datastore fails when saving entity with null attribute value #1022

toboid opened this issue Dec 14, 2015 · 11 comments
Assignees
Labels
api: datastore Issues related to the Datastore API.

Comments

@toboid
Copy link

toboid commented Dec 14, 2015

When trying to save an object that has null property values gcloud-node errors:

dataset.insert({
  key: dataset.key('Test'),
  data: { foo: 'bar', baz: null }
}, err => {
  if (err) return console.log(err);
});

prints:

Error: Unsupported field value, null, is provided.

I can see that the definition for Value in the Datastore proto buffers api does not provide for null values. The Ruby and Python clients create a message with no value attribute, is this an option for gcloud-node? I'd be really pleased to take a look at this if you'll take a pull request?

@stephenplusplus
Copy link
Contributor

Definitely, that would be very welcome!

// @pcostell - makes sense, right?

@stephenplusplus stephenplusplus added enhancement api: datastore Issues related to the Datastore API. labels Dec 14, 2015
@pcostell
Copy link
Contributor

Makes sense to me. Can we make sure this gets updated for the v1beta3 branch as well? In particular the next version of the API includes NullValue as an explicit field: https://github.com/google/googleapis/blob/master/google/datastore/v1beta3/entity.proto#L119

@toboid
Copy link
Author

toboid commented Dec 14, 2015

OK great. Correctly saving the null to datastore is straightforward, however when the entity is retrieved the null value comes back out as an empty array because this function isn't able to distinguish between a property representing an empty array and a property representing a null value since they have the exact same structure:

{
  "key": {
    ...
  },
  "property": [
    {
      "name": "i-am-a-null",
      "value": {
        "boolean_value": null,
        "integer_value": null,
        "double_value": null,
        "timestamp_microseconds_value": null,
        "key_value": null,
        "blob_key_value": null,
        "string_value": null,
        "blob_value": null,
        "entity_value": null,
        "list_value": [],
        "meaning": null,
        "indexed": true
      }
    },
    {
      "name": "i-am-an-empty-array",
      "value": {
        "boolean_value": null,
        "integer_value": null,
        "double_value": null,
        "timestamp_microseconds_value": null,
        "key_value": null,
        "blob_key_value": null,
        "string_value": null,
        "blob_value": null,
        "entity_value": null,
        "list_value": [],
        "meaning": null,
        "indexed": true
      }
    }
  ]
}

I'm still looking into how to handle this, do you have any suggestions? Should be easier for the v1beta3 branch.

@stephenplusplus
Copy link
Contributor

That makes for a tricky problem, indeed. Can we glean any insight into handling this from the other client libraries?

@stephenplusplus
Copy link
Contributor

@pcostell added nullValue support to the v1beta3 PR 👍

@mcfarljw
Copy link
Contributor

I'm also finding it a bit quirky to deal with nulls being returned as empty arrays.

@toboid
Copy link
Author

toboid commented Dec 20, 2015

I have the changes to correctly store null values here but still can't see a solution for getting them back out correctly at the moment

@pcostell
Copy link
Contributor

The Google versions of protobuf support this using the hasField method, which allows querying for whether a field has been explicitly set or not. There is an issue in the protobufjs issue tracker for this.

As mentioned above, this is fixed in v1beta3 (mostly because v1beta3 uses proto3, where unset and the default value are considered the same thing).

@stephenplusplus
Copy link
Contributor

Let's just wait for v1beta3 to have official support for this. It sounds like it's quite a hassle to support it now and v1beta3 is on the horizon. Support is ready in my v1beta3 PR, so I'm going to close the issue, but please re-open if there's something we can do to support it now.

@toboid
Copy link
Author

toboid commented Jan 4, 2016

Makes sense, I can't see a non-hacky way to correctly retrieve nulls at the moment.

@krisnye
Copy link

krisnye commented Jan 21, 2016

I have a fork which adds support for writing null values to the datastore today via a trivial fix. It does NOT fix the problem with reading them as empty arrays. (Not a problem for my use case.)

https://github.com/krisnye/gcloud-node

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API.
Projects
None yet
Development

No branches or pull requests

5 participants