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

Interval processing causes panics #49

Closed flamedmg closed 11 months ago

flamedmg commented 2 years ago

Hello, i'm getting the following when trying to pass interval to postgresql stored function:

panic: encode []ProductIn: cannot convert {31207680000000 0 0 2} to Interval

I tried to debug and found that error happens here: https://github.com/jackc/pgtype/blob/master/interval.go#L45 Object that is passed there is of type pgtype.Interval, which can't be processed properly as you can see. Could you please suggest how i can fix the issue and i will be happy to issue PR

jschaf commented 2 years ago

Can you post a minimal schema, query, and pggen invocation to reproduce?

Inferring from the error message, it looks like you have a custom type ProductIn used as input to pggen. It looks like you're passing an array, so []ProductIn, and you want to cast to the postgres type Interval[] but pggen is trying to cast to a single interval.

Could be a type mismatch in your pggen query or it could be a bug in pggen (handling input array elements wasn't the simplest code I've ever written).

I have a branch open that touches some of this. My plan was to tackle a bunch of open bugs over the break.

flamedmg commented 2 years ago

Here is the schema: schema.sql

CREATE TYPE public.test_in as
(
    sku              text,
    trial_interval   interval
);

create function public.test_in_fn(f_products public.test_in) returns void as
$$
begin
    select f_products;
end ;
$$ language plpgsql strict
                    volatile
                    security definer
                    set search_path to pg_catalog, public, pg_temp;

query.sql

--name: IntervalTest :exec
select public.test_in_fn(pggen.arg('test_in_arg')); 

code that calls IntervalTest function

    a := ""
    NewQuerier(p.db).IntervalTest(ctx, TestIn{
        Sku:           &a,
        TrialInterval: pgtype.Interval{Status: pgtype.Present, Microseconds: microsecondsPerHour * 1},
    })

In real code i'm passing an array of products, but this code fails as well.

jschaf commented 2 years ago

I can reproduce and looking for a fix.

jschaf commented 2 years ago

The panic occurs because we're setting a pgtype.Interval to another Interval which should be supported by pgtype. The relevant code is https://github.com/jackc/pgtype/blob/d846dbcb75b2ac38a1c6fd0390cd18472fe72dee/interval.go#L35:

    if value, ok := src.(interface{ Get() interface{} }); ok {
        value2 := value.Get()
        if value2 != value {
            return dst.Set(value2)
        }
    }

I don't understand why pgtype only returns if value2 != value. Seems like we can return nil if value2 == value.

Edit: oh, it's a check to see if the Get value is different than the ValueEncoder.

Here's a test case showing the problem:

func TestPgtype_Interval(t *testing.T) {
    v1 := pgtype.Interval{Days: 2, Status: pgtype.Present}
    v2 := pgtype.Interval{Days: 2, Status: pgtype.Present}
    err := v1.Set(v2)
    require.NoError(t, err) // Errors
}
flamedmg commented 1 year ago

I see it's still not closed and i'm using my forked pgtype version which properly assigns passed interval. It's basic 2 lines change and i see you are participating in pgtype repo. Could you please create pr for that change?

here is how i fixed it:

switch value := src.(type) {
    case time.Duration:
        *dst = Interval{Microseconds: int64(value) / 1000, Status: Present}
    case Interval:
        *dst = value
    default:
        if originalSrc, ok := underlyingPtrType(src); ok {
            return dst.Set(originalSrc)