jschaf / pggen

Generate type-safe Go for any Postgres query. If Postgres can run the query, pggen can generate code for it.
MIT License
282 stars 26 forks source link

Encoding of Numeric inside a custom type panics #39

Open tbo opened 3 years ago

tbo commented 3 years ago

I receive a panic when I try to insert entries with custom types containing Numeric fields. The relevant schema looks like this:

CREATE TYPE product_dimension AS (
  type TEXT,
  unit TEXT,
  value NUMERIC(8,2)
);

CREATE TABLE product (
  [...]
  dimensions product_dimension[],
  [...]
);

I get the following error message If I try to insert a row with value = 55.44:

panic encode []ProductDimension: cannot convert {5044 -2 2 false} to Numeric

I guess he is trying to convert a Numeric struct to a Numeric struct, but I might be wrong. This is the generated code that causes the panic:

func (tr *typeResolver) newProductDimensionArrayInit(ps []ProductDimension) pgtype.ValueTranscoder {
    dec := tr.newProductDimensionArray()
    if err := dec.Set(tr.newProductDimensionArrayRaw(ps)); err != nil {
        panic("encode []ProductDimension: " + err.Error()) // should always succeed
    }
    return textPreferrer{ValueTranscoder: dec, typeName: "_product_dimension"}
}

Considering that the comment says should always succeed I'm inclined to think that this might be a bug.

jschaf commented 3 years ago

Thanks for the detailed report. I can reproduce. The error is that pgtype.Numeric cannot be Set from another pgtype.Numeric instance. This doesn't occur with other numeric types because they're mapped to a literal type. For example, pgtype.Int8 uses the Go int64 scalar type.

panic encode []ProductDimension: cannot convert {5044 -2 2 false} to Numeric

That's the Go representation of a Numeric type, 5044e-2 (assuming you meant 50.44 instead of 55.44).

Here's the minimal reproducible case:

func TestNewQuerier_SetNumericType(t *testing.T) {
    num := &pgtype.Numeric{}
    require.NoError(t, num.Set(64))
    decoder := &pgtype.Numeric{}
    err := decoder.Set(num)
    require.NoError(t, err) // same error here
}

I'm not sure if this is intended behavior by pgtype. I expected that pgtype.Numeric could be Set from another pgtype.Numeric but I'll file a bug upstream to make sure.

As a workaround, I tried to set --gotype 'numeric=*string', so you could use a string type for numeric. That didn't work. I'll take a look at making that work since that's a clear bug in pggen.

jschaf commented 3 years ago

So, we're a bit stuck. Here's the options:

Short term, you're best bet is probably to rely on an external type and register the encoder and decoder: https://github.com/jackc/pgx/wiki/Numeric-and-decimal-support.

Medium term, --gotype numeric=*string should work for all cases but is blocked by merging a PR into pgtype for pgtype.Numeric.AssignTo. I'd like to avoid requiring an external library.

jschaf commented 3 years ago

I added an example of how to make this work with a real Decimal type in 7f87888. The main trick is to register the data type on the Querier:

    q := NewQuerierConfig(conn, QuerierConfig{
        DataTypes: []pgtype.DataType{
            {
                Value: &shopspring.Numeric{},
                Name:  "numeric",
                OID:   pgtype.NumericOID,
            },
        },
    })

And then, when invoking pggen, use: --go-type numeric=github.com/shopspring/decimal.Decimal.

The pgx recommendation below won't work because there's not a way to get registered DataTypes of all types of pgx connections, including pgx.Conn, and pgxpool.Pool. The pgx recommendation for pgxpool is to use an AfterConnect hook to call RegisterDataType but pggen can't "see" that occur.

// pgx way that won't work with pggen.
conn.ConnInfo().RegisterDataType(pgtype.DataType{
  Value: &shopspring.Numeric{},
  Name:  "numeric",
  OID:   pgtype.NumericOID,
})
tbo commented 3 years ago

@jschaf Thank you for your quick and detailed feedback. I was already using float64 as input, so the workaround with --gotype numeric=float64 didn't come with any additional loss of precision.