ryanrolds / sqlclosecheck

Linter that confirms DB rows and statements are closed properly.
MIT License
83 stars 15 forks source link

About the scope of the pgx linter #36

Open junsred opened 9 months ago

junsred commented 9 months ago

Pgx apparently closes rows when all rows are read and I guess because of this defer seems not needed. But this shouldn't be the case since function might stop before iterating all the rows. Is this a bug or feature?

    // Next prepares the next row for reading. It returns true if there is another
    // row and false if no more rows are available. It automatically closes rows
    // when all rows are read.
    Next() bool

Code below does not give any errs

var pgxPool *pgxpool.Pool
func missingClosePgxPool() {
    rows, err := pgxPool.Query(context.Background(), "SELECT username FROM users") // want "Rows/Stmt/NamedStmt was not closed"
    if err != nil {
        log.Fatal(err)
    }

    for rows.Next() {
        }
}
samsullivan commented 4 months ago

I'm curious on this one, too; CC @hallabro who worked on this originally in #25.

magnetikonline commented 3 months ago

Pgx apparently closes rows when all rows are read and I guess because of this defer seems not needed.

I'm seeing the same behaviour. Worthy to note this is not a feature of pgx - but default behaviour of database/sql.

Personally, I'd rather not see for rows.Next() {} be considered "OK" if that's the case - would rather have an defer rs.Close() in-place regardless - e.g. you might return inside the for loop/etc. making the defer pretty critical.

But this shouldn't be the case since function might stop before iterating all the rows.

exactly 👍