Retry logic can cause queries to be silently performed twice

driver.ErrBadConn should only be returned when it is safe to retry the query. lib/pq can return this error when the query has already been sent.

I ran across this because someone was requesting graceful handling of database restarts in https://github.com/jackc/pgx/issues/672. To me that is a very tricky problem to handle without sacrificing either safety or performance. But they said that lib/pq does what they want. So I took a quick look and it appears that safety is being sacrificed.

Below is an example. It runs a query that only inserts one record, but there are 2 records inserted because internally it is retried.

package main

import (
    "database/sql"
    "log"
    "os"

    _ "github.com/lib/pq"
)

func main() {
    db, err := sql.Open("postgres", os.Getenv("DATABASE_URL"))
    if err != nil {
        log.Fatal(err)
    }

    _, err = db.Exec("drop table if exists t")
    if err != nil {
        log.Fatal(err)
    }

    _, err = db.Exec("create table t (id int primary key generated by default as identity)")
    if err != nil {
        log.Fatal(err)
    }

    _, err = db.Exec(`
begin;
insert into t(id) values (default);
commit;
select pg_terminate_backend(pg_backend_pid()) where 1 = (select count(*) from t);
`)
    if err != nil {
        log.Fatal(err)
    }

    var numInserts int
    err = db.QueryRow("select count(*) from t").Scan(&numInserts)
    if err != nil {
        log.Fatal(err)
    }

    log.Println("expected 1 row inserted, got", numInserts)
}

jack@glados ~/dev/pqretrybug » PGHOST=/private/tmp go run main.go
2020/02/01 15:56:40 expected 1 row inserted, got 2

lib / pq

Retry logic can cause queries to be silently performed twice #939