Google-cloud-go: datastore: allow sending data from nil pointers

[eriklott]

The current go datastore client does not support saving pointer struct fields.

type Foo struct {
    Bar *string `datastore:"bar"`
    Baz *int `datastore:"baz,omitempty"`
}

In the example above, if both Bar and Baz are nil pointers, I would expect Bar to be stored as NULL, and Baz to be omitted. Is there a technical reason why the current client doesn't support this? Thanks.

[odeke-em]

@eriklott I've retitled the issue a little bit.

In the https://godoc.org/google.golang.org/api/datastore/v1 and other APIs from google.golang.org/api/* I've seen the ability for user to set the ForceSendFields []string field for calls to explicitly serialize and transmit nil values. I'll defer to @zombiezen @jba if there are options like this for the protobuf generated APIs.

[eriklott]

@odeke-em better title :) I'm not familiar with the ForceSendFields []string from other packages, but I think the cloud.google.com/go/datastore package api works as it is - it just needs a little internal massaging to support pointers. Feel free to correct me.

For example, the package already supports NULL values in regards to the datastore.Key type:

type Foo struct {
   Bar *datastore.Key
}

When Foo.Bar is nil, a NULL value will be transmitted to the datastore.

https://github.com/GoogleCloudPlatform/google-cloud-go/blob/master/datastore/save.go#L353

It would be idiomatic to extend support to primitive pointers, and primitive based custom types as well, while using omitempty to control if we'd like to represent a nil pointer value with NULL or omit the value completely.

type Roo string

type Foo struct {
    A *string `datastore:",omitempty"` // not transmitted when nil
    B *int  // transmits NULL when nil
    C *Roo `datastore:",omitempty"` // not transmitted when nil
}

[zombiezen]

@adams-sarah, can you take a look?

[adams-sarah]

sorry for my delay
nope, there's no technical reason why we don't allow pointers to non-struct types.
i actually had support for this in a CL somewhere, but removed the change to add back later. never got around to it.

it would be fairly simple to add. there is a lot of recursion going on in loading/saving, so you'd just dereference the pointer and recurse again. should be straight forward.

happy to review the CL! add [email protected]

[jba]

Closing this. Our focus is now on Firestore, which I believe has this feature.

[lc-chrisbarton]

Well I guess my use case is not possible and sure I can work around it by adding extra logic but it would be nice to support it and it does work if I use Javascript. I would like to stop money and time data being added to Datastore if it was never set to a non-default value in the struct? E.g. persisting an order with the DateComplete and Paid fields not set until they really are? omitempty doesn't work with the default value for time and we could zero rate an order

type Order struct {
Status string
DateReceived time.Time
DateComplete time.Time
Paid float64
}

[elithrar]

Closing this. Our focus is now on Firestore, which I believe has this feature.

@jba @broady To confirm, as someone building a service on App Engine Flex: should I double-down on Firestore + Firestore client libraries going forward?

Right now the API surface between Go client libs isn't too different, but the above comment makes it sound like Firestore is going to see (all of?) the future development work.

[lc-chrisbarton]

Interesting comment @elithrar should we be using Firestore instead of Datastore?

[lc-chrisbarton]

It's a pity that @adams-sarah mentions that is it simple to add support for omitting attributes from Datastore into the go sdk but the decision was taken not to support this and instead create a new sdk for Firestore with this feature. I think this is a pretty basic feature of a database and in my second day of development I found myself needing it :(

[zombiezen]

@ChrisBartonLC: it's a matter of rolling out and compatibility. The individual code change may be simple, but it must be rolled out to (and not break users of) cloud.google.com/go/datastore, google.golang.org/appengine/datastore, and appengine/datastore. That part is more complex and has many more dependencies.

[lc-chrisbarton]

Thanks @zombiezen, I guess add firestore to that list too now :( The disadvantage of tight coupling, it slows you down and even when releases happen its hard not to break stuff, see recent Node.js datastore release 1.2

[lc-chrisbarton]

It is an excellent api though, just a pity that crucial feature is missing. I don't really understand why since I often use null/missing values to indicate stuff not being deliberately set, I have been working in Node.js a lot lately, perhaps that's why...

[dmcgrath]

A quick note to correct "Our focus is now on Firestore" which was the result of some internal miscommunication.

I want to clarify that this is still supported and both Cloud Datastore (server-side development centric) and Cloud Firestore (mobile development centric) live side by side.

[dustinmoris]

Given that Google operates Firestore in Datastore mode and even recommends to use this mode in server side scenarios according to the official Google Cloud documentation is it too much of an ask to re-open this issue and show the cloud.google.com/go/datastore package some love?