jackc / pgtype

MIT License
308 stars 110 forks source link

Panic when a NaN is present #169

Closed ghstahl closed 2 years ago

ghstahl commented 2 years ago

I have a postgres table

CREATE TABLE IF NOT EXISTS public.point_series
(
    point_id character varying(36) COLLATE pg_catalog."default" NOT NULL,
    timestamp_utc timestamp with time zone NOT NULL,
    timestamp_local timestamp without time zone NOT NULL,
    double_value double precision,
    org_id character varying(50) COLLATE pg_catalog."default" NOT NULL DEFAULT ''::character varying,
    geoshape geometry
)

with a double value

I can insert a NaN into the double_value

insert into point_series(org_id, point_id, timestamp_utc, timestamp_local, double_value, geoshape) values ('org','point','2022-06-23 15:01:03-08' ,'2019-10-11 10:10:25-07','NaN',null)

from the documentation it is allowed https://www.postgresql.org/docs/9.3/datatype-numeric.html

The call originates from github.com/jackc/pgx

When I do query it panics Here is the stack dump

{

    "stack": [{
        "func": "(*service).List.func2",
        "line": "107",
        "source": "timeseries_service.go"
    }, {
        "func": "gopanic",
        "line": "844",
        "source": "panic.go"
    }, {
        "func": "panicmem",
        "line": "220",
        "source": "panic.go"
    }, {
        "func": "sigpanic",
        "line": "255",
        "source": "signal_windows.go"
    }, {
        "func": "(*Int).Set",
        "line": "75",
        "source": "int.go"
    }, {
        "func": "NewFromBigInt",
        "line": "116",
        "source": "decimal.go"
    }, {
        "func": "(*Numeric).DecodeBinary",
        "line": "266",
        "source": "decimal.go"
    }, {
        "func": "scanPlanDstBinaryDecoder.Scan",
        "line": "513",
        "source": "pgtype.go"
    }, {
        "func": "(*connRows).Scan",
        "line": "224",
        "source": "rows.go"
    }, {
        "func": "(*poolRows).Scan",
        "line": "70",
        "source": "rows.go"
    }, {
        "func": "(*service).List",
        "line": "187",
        "source": "timeseries_service.go"
    }, {
        "func": "(*service).List",
        "line": "94",
        "source": "timeseries_service.go"
    }, {
        "func": "Value.call",
        "line": "556",
        "source": "value.go"
    }, {
        "func": "Value.Call",
        "line": "339",
        "source": "value.go"
    }, {
        "func": "(*Runtime).executeDirect",
        "line": "519",
        "source": "runtime.go"
    }, {
        "func": "(*Runtime).ExecuteWorkflow",
        "line": "419",
        "source": "runtime.go"
    }, {
        "func": "(*TimeseriesServiceState).List",
        "line": "103",
        "source": "timeseries_gtm.pb.go"
    }, {
        "func": "_TimeseriesService_List_Handler.func1",
        "line": "132",
        "source": "timeseries_grpc.pb.go"
    }, {
        "func": "UnaryServerInterceptor.func1",
        "line": "33",
        "source": "interceptors.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "AuthUnaryServerInterceptor.func1",
        "line": "50",
        "source": "authinterceptor.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "LoggingUnaryServerInterceptor.func1",
        "line": "31",
        "source": "logginginterceptor.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "MetadataFilterUnaryServerInterceptor.func1",
        "line": "48",
        "source": "metadatafilterinterceptor.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "EnsureCorrelationIDUnaryServerInterceptor.func1",
        "line": "124",
        "source": "correlationIDInterceptor.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "EnsureContextLoggingUnaryServerInterceptor.func1",
        "line": "21",
        "source": "logginginterceptor.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }, {
        "func": "UnaryServerInterceptor.func1",
        "line": "23",
        "source": "interceptors.go"
    }, {
        "func": "ChainUnaryServer.func1.1.1",
        "line": "25",
        "source": "chain.go"
    }],
    "error": "panic: runtime error: invalid memory address or nil pointer dereference",

    "time": 1656022799,
    "caller": "C:/work/github/mapped/ccc/ts-micro/internal/service/timeseries/postgres/timeseries_service.go:108"
}  

The code that is causing it https://github.com/jackc/pgtype/blob/6dd004c8b8f4f938a26778020882139b8f4de1c2/ext/shopspring-numeric/decimal.go#L262

func (dst *Numeric) DecodeBinary(ci *pgtype.ConnInfo, src []byte) error {
    if src == nil {
        *dst = Numeric{Status: pgtype.Null}
        return nil
    }

    // For now at least, implement this in terms of pgtype.Numeric

    num := &pgtype.Numeric{}
    if err := num.DecodeBinary(ci, src); err != nil {
        return err
    }

    *dst = Numeric{Decimal: decimal.NewFromBigInt(num.Int, num.Exp), Status: pgtype.Present}

    return nil
}

The num.DecodeBinary looks like its doing the right thing and sets internal values

Int: *math/big.Int nil
Exp: 0
Status: Present (2) = 0x0
NaN: true
InfinityModifier: None (0)

Then it proceeds to pass a nil to decimal.NewFromBigInt(...). i.e. num.Int is nil.

Should there be a NaN == true check here?

jackc commented 2 years ago

Fixed.