I am having trouble with scanning from a pgx query in Golang. The id field is always that of the last record. If I un-comment the var person Person declaration at the top of the function, every id is 3. There are 3 records with id's from 1 to 3 in my db. When I comment that declaration and declare the variable in the rows.Next() loop I get the correct id's. I can't figure out why the personId isn't being correctly overwritten

output from marshalled JSON with the var declared at the top of the function.

[{"person_id":3,"first_name":"Mark","last_name":"Brown"},{"person_id":3,"first_name":"Sam","last_name":"Smith"},{"person_id":3,"first_name":"Bob","last_name":"Jones"}]

output after declaring person every iteration of the scan loop

[{"person_id":1,"first_name":"Mark","last_name":"Brown"},{"person_id":2,"first_name":"Sam","last_name":"Smith"},{"person_id":3,"first_name":"Bob","last_name":"Jones"}]

I have this struct

// Person model
type Person struct {
    PersonId       *int64   `json:"person_id"`
    FirstName      *string  `json:"first_name"`
    LastName       *string  `json:"last_name"`
}

Here is my query function

func getPersons(rs *appResource, companyId int64) ([]Person, error) {

    //  var person Person

    var persons []Person

    queryString := `SELECT 
      user_id, 
      first_name, 
      last_name,
      FROM users 
      WHERE company_id = $1`

    rows, err := rs.db.Query(context.Background(), queryString, companyId)

    if err != nil {
        return persons, err
    }

    for rows.Next() {

        var person Person

        err = rows.Scan(
            &person.PersonId,
            &person.FirstName,
            &person.LastName)

        if err != nil {
            return persons, err
        }

        log.Println(*person.PersonId) // 1, 2, 3 for both var patterns

        persons = append(persons, person)
    }

    if rows.Err() != nil {
        return persons, rows.Err()
    }

    return persons, err
}

CodePudding user response：

I believe that you have discovered a bug (or, at least, unexpected behaviour) in github.com/jackc/pgx/v4. When running Scan it appears that if the pointer (so person.PersonId) is not nil then whatever it is pointing to is being reused. To prove this I replicated the issue and confirmed that you can also fix it with:

persons = append(persons, person)
person.PersonId = nil

I can duplicate the issue with this simplified code:

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). I have had a quick look at the issues and could not find anything reported (will have a more detailed look later).