Retry logic can cause queries to be silently performed twice

[jackc]

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 jackc/pgx#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

[andreimatei]

I think this issues was supposed to have been fixed in #730 - but apparently it hasn't?
CC @mjibson