sqlboiler icon indicating copy to clipboard operation
sqlboiler copied to clipboard

Insert fails with SIGSEGV when a table has numeric column with not null constraint

Open mtsmfm opened this issue 5 years ago • 3 comments

The following code fails with SIGSEGV

	user := models.User{}
	user.Insert(ctx, db, boil.Infer())
$ DATABASE_URL=postgres://postgres:password@postgres/postgres?sslmode=disable go run utils/foo.go          
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0x68e9dc]

goroutine 1 [running]:
github.com/ericlagergren/decimal.(*Big).IsNaN(...)
        /home/app/go/pkg/mod/github.com/ericlagergren/[email protected]/big.go:853
github.com/volatiletech/sqlboiler/v4/types.decimalValue(0x0, 0x8fe300, 0x0, 0x0, 0x7fa1a03ebca0, 0x8a9601)
        /home/app/go/pkg/mod/github.com/volatiletech/sqlboiler/[email protected]/types/decimal.go:140 +0x3c
github.com/volatiletech/sqlboiler/v4/types.Decimal.Value(0x0, 0xc0001a15f0, 0x40d1e5, 0x6c4760, 0x7007c0)
        /home/app/go/pkg/mod/github.com/volatiletech/sqlboiler/[email protected]/types/decimal.go:58 +0x30

schema

CREATE TABLE IF NOT EXISTS users (
  id INTEGER PRIMARY KEY,
  dollars NUMERIC NOT NULL
);

https://gist.github.com/mtsmfm/738f16a88a0104d4f40692d68f30d68e

Is this intentional?

mtsmfm avatar Oct 11 '20 10:10 mtsmfm

The decimal type is a bit weird. Because you have not null there, it expects you to have a value for it. Semi-intentional.

The decimal type is defined like this:

type Decimal struct {
  *big.Decimal
}

so if the big.Decimal is nil it'll panic like this.

I don't know if it's fixable, but I'd accept a PR to do a check and return an error if it is.

aarondl avatar Nov 04 '20 04:11 aarondl

This was fixed by #1044. And released in v4.8.4

The code above will now throw an error for violating the not-null constraint.

But this raises a new question:
Should the driver.Value() of a new types.Decimal{} be 0.0? Currently, it would return nil which the driver would report as NULL.

We have explicit types.Decimal and types.NullDecimal and since we know that types.Decimal is not meant to be NULL should we prevent this by default?

stephenafamo avatar Jan 28 '22 04:01 stephenafamo

PR: #1073

stephenafamo avatar Jan 28 '22 05:01 stephenafamo