mgutz / dat

Go Postgres Data Access Toolkit
Other
612 stars 62 forks source link

maxLookup index out of range on update #54

Open lgonzalez-silen opened 7 years ago

lgonzalez-silen commented 7 years ago

Hi,

Upon adding a new column to a table that was already large I started experiencing index out of range at

https://github.com/mgutz/dat/blob/v1/update.go#L178

It seems the condition that triggers hand off between preallocated parameter conversions and ad hoc conversions may be off by one. If I change i < maxLookup to i < maxLookup - 1 everything works fine but not sure if that's right, as the last element in equalsPlaceholderTab will not be used. Also, is it right that equalsPlaceholderTab starts with $0?

I wrote a little snippet to test for various values of maxLookup (https://github.com/mgutz/dat/blob/v1/init.go#L19):

type TestStruct struct {
    ID     int64  `db:"id"`
    Field1 string `db:"field1"`
    Field2 string `db:"field2"`
}

func TestBreak() {
    DB.SQL(`CREATE TABLE testtable (id serial, field1 text, field2 text);`).Exec()
    DB.SQL(`INSERT INTO testtable (field1, field2, field3) VALUES ('a', 'b');`).Exec()

    record := TestStruct{}
    _, err := DB.
        Update("testtable").
        SetWhitelist(record, "*").
        Where("id = $1", 1).
        Exec()
    if err != nil {
        panic(err)
    }

    DB.SQL(`DROP TABLE testtable;`).Exec()
}

This breaks for maxLookup <= 3 but works for larger values.

Cheers,

Luis

mgutz commented 7 years ago

Not ignoring this. I'm in a bit of time crunch right now.

lgonzalez-silen commented 7 years ago

np! thanks

lgonzalez-silen commented 7 years ago

See the open PR referenced above. Since we use our large table extensively, it might still be good to update maxLookup for us (although probably very marginally), but in general the PR fixes the bug.