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

Spanner: Support Custom Marsheling and Unmarsheling #854

Closed
hostirosti opened this issue Jan 9, 2018 · 5 comments · Fixed by #2614
Closed

Spanner: Support Custom Marsheling and Unmarsheling #854

hostirosti opened this issue Jan 9, 2018 · 5 comments · Fixed by #2614
Assignees
Labels
api: spanner Issues related to the Spanner API. help wanted We'd love to have community involvement on this issue. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@hostirosti
Copy link

It would be great to have the possibility to have custom un-/marsheling for custom types support a la JSON de-&encoder or CSV de-&encoder:

Example:

DDL:

CREATE TABLE ProductInfo (
    ProductID    INT64 NOT NULL,
    UpdateDate   TIMESTAMP NOT NULL,
    Data         STRING
) PRIMARY KEY (ProductID)
type customField struct {
  Prefix string
  Suffix string
}
type dbProductInfo struct {
    ProductID  int64       `spanner:"ProductID"`
    UpdateDate time.Time   `spanner:"UpdateDate"`
    Data       customField `spanner:"Data"`
}

// Convert the customField 
func (cf *customField) MarshalSpanner() (string, error) {
    var b bytes.Buffer
    b.WriteString(cf.Prefix)
    b.WriteString("-")
    b.WriteString(cf.Suffix)
    return b.String(), nil
}

// Convert Spanner string to customField
func (cf *customField) UnmarshalSpanner(val string) (err error) {
   s := strings.SplitAfter(val, "-")
   if len(s) > 1 {
       cf.Prefix = s[0]
       cf.Suffix = s[1]
   }
   return nil
}
@jba jba self-assigned this Jan 11, 2018
@jba jba added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. api: spanner Issues related to the Spanner API. labels Jan 11, 2018
@danoscarmike danoscarmike removed the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Jun 6, 2018
@jba
Copy link
Contributor

jba commented Aug 16, 2018

First, I'm not sure if this is something we want to support.

If we do, however, string seems like the wrong type for this. Spanner strings must be UTF-8. Maybe []byte. But for that, we have BinaryMarshaler.

So maybe we could test if a type implements BinaryMarshaler/Unmarshaler to write/read from a BYTES column.

@jeanbza
Copy link
Member

jeanbza commented Oct 2, 2019

@olavloite WDYT about this issue? Is this reasonable, or do you think it's out of scope?

@olavloite
Copy link
Contributor

It's a nice-to-have, and I can understand the wish considering we already support automatically converting a row to a struct and a struct to a row (for updates and inserts). Limiting that process to purely copying data directly from one column to a field with the exact same type is quite limiting, and allowing some customization of that mapping process is reasonable.

I would however take a slightly more generic approach and allow any valid Spanner data type to be encoded and decoded to/from any Go data type, and not limit it to []byte and/or string.

That would also enable automatic encoding and decoding of for example decimal values, for which Spanner does not have a native data type. The Spanner documentation recommends encoding decimal values in either STRING or INT64 data types.

So we would get two interfaces approximately like this:

type SpannerEncoder interface {
	EncodeSpanner() (interface{}, error)
}

type SpannerDecoder interface {
	DecodeSpanner(input interface{}) error
}

@jeanbza jeanbza added the help wanted We'd love to have community involvement on this issue. label Oct 10, 2019
@hengfengli
Copy link
Contributor

@olavloite, I am investigating this issue now. I tried the following example:

package main

import (
	"context"
	"log"
	"time"

	"cloud.google.com/go/spanner"
)

type customField struct {
	Prefix string
	Suffix string
}
type dbProductInfo struct {
	ProductID  int64       `spanner:"ProductID"`
	UpdateDate time.Time   `spanner:"UpdateDate"`
	Data       customField `spanner:"Data"`
}

func main() {
	ctx := context.Background()
	client, err := spanner.NewClient(ctx, "DB_PATH")
	if err != nil {
		log.Fatal(err)
	}
	defer client.Close()

	d := dbProductInfo{
		ProductID:  1234,
		UpdateDate: time.Now(),
		Data:       customField{"prefix", "suffix"},
	}

	m, err := spanner.InsertStruct("ProductInfo", &d)
	if err != nil {
		log.Fatalf("err: %v", err)
	}

	ms := []*spanner.Mutation{m}
	_, err = client.Apply(ctx, ms)
	if err != nil {
		log.Fatalf("err: %v", err)
	}
}

But it throws an error: 2020/07/20 13:23:07 err: spanner: code = "InvalidArgument", desc = "client doesn't support type main.customField" from:

if !isSupportedMutationType(v) {
return nil, errEncoderUnsupportedType(v)

It can support custom base types, e.g., once I changed to:

type customField string

It works fine.

How can your proposed interfaces apply here? I guess we expect customField should implement EncodeSpanner so we can convert it to a base type. Similar for decoding process.

@olavloite
Copy link
Contributor

How can your proposed interfaces apply here? I guess we expect customField should implement EncodeSpanner so we can convert it to a base type. Similar for decoding process.

Yes, that is the basic idea, and that could also apply to custom structs. So if a struct implements the SpannerEncoder interface and is used as a field in a data struct, it should be recognized as such by the client library, which would then call the EncodeSpanner method that should return a valid base type for a Spanner column. That could be any of the built-in go types that are supported by the client library, or a custom type that points back to a supported built-in type.

In the initial example of this issue the struct is encoded to a string, but ideally we should allow the struct to be encoded to any type that is supported by the client library. When this issue was originally filed, that meant only one of the built-in types (string, int64, float64, etc.). Now that the client library also supports custom types that point back to the built-in base types, this feature should also support those types.

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. help wanted We'd love to have community involvement on this issue. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants