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

types.NullDecimal with nil causes panic #404

Closed
andradei opened this issue Oct 23, 2018 · 12 comments
Closed

types.NullDecimal with nil causes panic #404

andradei opened this issue Oct 23, 2018 · 12 comments
Labels

Comments

@andradei
Copy link

andradei commented Oct 23, 2018

Not a generation problem.

What version of SQLBoiler are you using (sqlboiler --version)?

v3.0.1

If this happened at generation time what was the full SQLBoiler command you used to generate your models? (if not applicable leave blank)

No

If this happened at runtime what code produced the issue? (if not applicable leave blank)

type MyStruct {
    Price types.NullDecimal
}

ms := &MyStruct{
    Price: types.NewNullDecimal(nil)
}

// This prints
// &MyStruct{
//     Price: Price:%!v(PANIC=runtime error: invalid memory address or nil pointer dereference)
// }
fmt.Printf("%#v\n", ms)

b, _ := json.Marshal(&ms) // panics here

What is the output

runtime error: invalid memory address or nil pointer dereference

Further information. What did you do, what did you expect?

It is expected that d := types.NewNullDecimal(nil) or d := NullDecimal{} or var d NullDecimal follow what its documentation states:

// NullDecimal is the same as Decimal, but allows the Big pointer to be nil.
// See docmentation for Decimal for more details.
//
// When going into a database, if Big is nil it's value will be "null".
type NullDecimal struct {
	*decimal.Big
}
@andradei andradei changed the title Marshalling types.NullDecimal with nil causes panic types.NullDecimal with nil causes panic Oct 23, 2018
@aarondl
Copy link
Member

aarondl commented Oct 30, 2018

Hey there. This is a problem in the decimal library that we use. I've created an issue over there (you can see it referenced above) to see if we can't get a fix.

@andradei
Copy link
Author

andradei commented Nov 14, 2018

Since the author cautioned against using "null" types with his decimal.Big for JSON formatting, could the support for nil be coded in this package instead?

Could it be extracted to the null package and have a similar API with a struct field called Valid?

@aarondl
Copy link
Member

aarondl commented Nov 15, 2018

Yeah. It seems like maybe what we should do is add MarshalJSON/UnmarshalJSON to NullDecimal directly. Since it's a value type it will always be able to do the correct thing.

@andradei
Copy link
Author

andradei commented Dec 11, 2018

The linked issue was closed, so one less panic to worry about. I don't want to commit to submitting a PR yet, but will look for time to work on this today or tomorrow and give an update here.

@aarondl
Copy link
Member

aarondl commented Mar 18, 2019

cc @AlexDunmow

This is obviously still an issue. I'm actually wondering if we could go against the grain and use decimal.Big as a value type instead. It makes working with it more annoying since you'd have & wherever you use it as an input, but if you use it to start a calculation there should be no change thanks to go's automatic address of when using .

The pointer was a primary concern when I was choosing a decimal type to use for sqlboiler, I'm sad to see that the choice indeed is coming back to bite me.

For people who don't care and just want to work around the problem, you could probably type replace with https://github.com/shopspring/decimal which doesn't use a pointer and be okay.

@andradei
Copy link
Author

Usually, marshalling/unmarshalling already follows an API where you use references (&Type). So this should be a familiar API usage. I like that idea.

Sorry I haven't got around to look at it as I said. Life got super busy when I was supposed to be on vacation.

@AlexDunmow
Copy link

In my opinion, https://github.com/shopspring/decimal is the superior library, especially when having to do calculations.

@andradei
Copy link
Author

Interesting. It implements JSON marshalling/unmarshalling too. Maybe that isn't a concern, but the last commit was one year ago. Solid libraries usually get at those stages.

@aarondl aarondl added the bug label Aug 26, 2019
@aarondl
Copy link
Member

aarondl commented Aug 26, 2019

Probably going to move to shopspring decimal in v4

@aarondl
Copy link
Member

aarondl commented Oct 9, 2019

This should actually be fixed on dev as soon as I push today's changes.

@aarondl aarondl closed this as completed Oct 9, 2019
@uded
Copy link

uded commented Mar 13, 2021

Interestingly, I am running v4 and still have this issue.

I would support @AlexDunmow with the idea to migrate to a better library.

Especially since the original decimal library seems to have some other problems like too-new and broken version for a while. Like for two years now if I am not mistaken...


My model:

// SourcePhistat is an object representing the database table.
type Source struct {
	ID                        int               `boil:"id" json:"id" toml:"id" yaml:"id"`
	Score                     types.NullDecimal `boil:"score" json:"score,omitempty" toml:"score" yaml:"score,omitempty"`

	R *sourcePhistatR `boil:"-" json:"-" toml:"-" yaml:"-"`
	L sourcePhistatL  `boil:"-" json:"-" toml:"-" yaml:"-"`
}

Debug output:

INSERT INTO "source" ("id", "score") VALUES ($1,$2) ON CONFLICT ("id") DO UPDATE SET "score" = EXCLUDED."score"
[6971153 %!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference)]

Model and output were simplified for exemplary purposes.

@aarondl
Copy link
Member

aarondl commented Mar 14, 2021

I think the issue is I never did push the bump to the version on the dev branch. My apologies. It should be there now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants