Unexpected Scan behaviour - reusing pointer

[MattBrittan]

This issue is from a stack overflow question. I guess this could be classed as "unexpected behaviour" but as it deviates from the standard library I believe it's probably unintended.

It's fairly common to pass Scan a pointer to a pointer (so that null results in a nil pointer`). However, it appears that if the pointer is not nil then whatever it is pointing to is being reused. This is probably best explained with an example:

conn, err := pgx.Connect(context.Background(), "postgresql://user:[email protected]:5432/schema?sslmode=disable")
if err != nil {
    panic(err)
}
defer conn.Close(context.Background())

queryString := `SELECT num::int FROM generate_series(1, 3) num`

var scanDst *int64
var slc []*int64

rows, err := conn.Query(context.Background(), queryString)

if err != nil {
    panic(err)
}

for rows.Next() {
    err = rows.Scan(&scanDst)

    if err != nil {
        panic(err)
    }

    slc = append(slc, scanDst)
    // scanDst = nil
}

if rows.Err() != nil {
    panic(err)
}

for _, i := range slc {
    fmt.Printf("%v %d\n", i, *i)
}

The output from this is:

0xc00009f168 3
0xc00009f168 3
0xc00009f168 3

You will note that the pointer is the same in each case. I have done some further testing:

• Uncommenting scanDst = nil in the above fixes the issue.
• When using database/sql (with the "github.com/jackc/pgx/stdlib" driver) the code works as expected.
• If PersonId is *string (and query uses num::text) it works as expected.

The issue appears to boil down to the following in convert.go:

if v := reflect.ValueOf(dst); v.Kind() == reflect.Ptr {
    el := v.Elem()
    switch el.Kind() {
    // if dst is a pointer to pointer, strip the pointer and try again
    case reflect.Ptr:
        if el.IsNil() {
            // allocate destination
            el.Set(reflect.New(el.Type().Elem()))
        }
        return int64AssignTo(srcVal, srcStatus, el.Interface())

So this handles the case where the destination is a pointer (for some datatypes). The code checks if it is nil and, if so, creates a new instance of the relevant type as a destination. If it's not nil it just reuses the pointer. Note: I've not used reflect for a while so there may be issues with my interpretation.

As the behaviour differs from database/sql and is likely to cause confusion I believe it's probably a bug (I guess it could be an attempt to reduce allocations).

[jackc]

As the behaviour differs from database/sql and is likely to cause confusion I believe it's probably a bug (I guess it could be an attempt to reduce allocations).

It was intentional, but perhaps it is a bit too surprising. I do try to match database/sql behavior when reasonable. I'm okay with changing this.

[markhorrocks]

Additionally, I am unable to use pgtype.Date with a pointer.

[jflambert]

just ran into this with v5.4.3