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

Utilize MarshalJSON and UnmarshalJSON interface implementations #503

Open
klokare opened this issue Jun 29, 2022 · 2 comments
Open

Utilize MarshalJSON and UnmarshalJSON interface implementations #503

klokare opened this issue Jun 29, 2022 · 2 comments

Comments

@klokare
Copy link

klokare commented Jun 29, 2022

Is your feature request related to a problem? Please describe.
Custom types can sometimes produce empty values in a RethinkDB document. I have implemented my own decimal type:

type Decimal struct {
	flags uint32
	high  uint32
	low   uint32
	mid   uint32
}

// MarshalJSON returns the decimal as a text string without quotes
func (d Decimal) MarshalJSON() ([]byte, error) { return d.MarshalText() }

// MarshalText encodes the receiver into UTF-8-encoded text and returns the result.
func (d Decimal) MarshalText() (text []byte, err error) {
	text = []byte(d.String())
	return text, nil
}

// UnmarshalJSON unmarshals the JSON value, ignoring quotes
func (d *Decimal) UnmarshalJSON(text []byte) error {
	return d.UnmarshalText(text)

}

// UnmarshalText unmarshals the decimal from the provided text.
func (d *Decimal) UnmarshalText(text []byte) (err error) {
	*d, err = Parse(string(text))
	return err
}

It implements both json.Marshaler and json.Unmarshaler. This type encodes and decodes without issue using the standard encoding/json package. So I was surprised to see the following document stored in RethinkDB

{
"candle": {
"bar_seqno": 12024547 ,
"close_price": { } ,
"high_price": { } ,
"low_price": { } ,
"open_price": { } ,
}
....
}

when using

type Candle struct {
  BarSeqno int `json:"bar_seqno"`
  OpenPrice Decimal `json:"open_price"`
  HighPrice Decimal `json:"high_price"`
  LowPrice Decimal `json:"low_price"`
  ClosePrice Decimal `json:"close_price"`
}

candle := Candle{
  BarSeqno: 12024547,
  OpenPrice: decimal.NewFromString("1.33028"),
  HighPrice: decimal.NewFromString("1.33028"),
  LowPrice: decimal.NewFromString("1.33028"),
  ClosePrice: decimal.NewFromString("1.33028"),
}

err := r.Table("candles").Insert(candle).Exec(session)

What I would expect to see is

{
"candle": {
"bar_seqno": 12024547 ,
"close_price": 1.33028 ,
"high_price": 1.33028 ,
"low_price": 1.33028 ,
"open_price": 1.33028 ,
}
....
}

Describe the solution you'd like
If this library could use the json.Marshaler and json.Unmarshaler implementations, I would get the expected value by just using

err := r.Table("candles").Insert(candle).Exec(session)

Describe alternatives you've considered
My workaround comes from this issue and is basically:

candles = []Candle{candle1, candle2, candle3}
b := new(bytes.Buffer)
	for _, candle := range candles {
		if err = json.NewEncoder(b).Encode(candle); err != nil {
			return err
		}
		if err = r.Table(name).Insert(r.JSON(b.String())).Exec(session); err != nil {
			return err
		}
		b.Reset()
	}

This is not only more verbose but also creates separate calls to Insert instead of sending a batch which impacts performance and also eliminates the transaction-like quality of sending a slice of objects to Insert.

Additional context
Is there something about RethinkDB or this library that would prevent adding this functionality? I would be happy to give it a try but not if someone has already proven it is a bad idea.

@klokare
Copy link
Author

klokare commented Jun 29, 2022

Update on the performance implications of the workaround above. Note: These tests are from my laptop in France to a hosted server in Germany. The raw values might not reflect on-server performance, but the relative values demonstrates the performance problem.

existing behaviour, which strips out the decimal values

t0 := time.Now()
err = rethinkdb.Table(name).Insert(candles[:100]).Exec(session)
d := time.Since(t0)
log.WithField("duration", d.Seconds()).Info("test")

time to insert 100 records is 0.23339381 seconds

using above work around

t0 := time.Now()
	for _, candle := range candles[:100] {
		if err = json.NewEncoder(b).Encode(candle); err != nil {
			return err
		}
		if err = rethinkdb.Table(name).Insert(rethinkdb.JSON(b.String())).Exec(session); err != nil {
			return err
		}
		b.Reset()
	}

d := time.Since(t0)
log.WithField("duration", d.Seconds()).Info("test")

time to insert 100 records is 5.65757508 seconds

batching the encoding

I also tried the following

	b := new(bytes.Buffer)
	bufs := make([]string, 100)
	for i, task := range tasks[:100] {
		if err = json.NewEncoder(b).Encode(task); err != nil {
			return err
		}
		bufs[i] = b.String()
		b.Reset()
	}
	t0 := time.Now()
	err = rethinkdb.Table(name).Insert(bufs[:100]).Exec(session)
	d := time.Since(t0)
	log.WithField("duration", d.Seconds()).Info("test")

to see if the slow down was in the encoding, or the rethinkdb-go actions. Interestingly, no documents were created in the RethinkDB database yet no error was given.

@klokare
Copy link
Author

klokare commented Jun 29, 2022

Came up with another workaround: create a separate struct just for inserting and retrieving objects into RethinkDB. Basically, a copy of the original struct with each Decimal changed to string and then having to allocate a new slice of objects and loop through, creating a copy of the original struct, transforming Decimals into strings. Very wordy, not ideal. But this app is small and it gets me back to the original performance and I can use a single Insert. Long-term though, the lack of MarshalJSON does make RethinkDB a less likely option for the other projects I work on as some of the types are third-party and the fields are not exposed so my only option would be the r.JSON() route.

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

No branches or pull requests

1 participant