Make Select reset slice length

[cespare]

sqlx's Select functions take a destination slice and fill it in. It's not documented, but they simply append to the slice; if it already had some elements, they are left alone.

Here's a demo:

package main

import (
	"encoding/json"
	"fmt"
	"log"

	"github.com/jmoiron/sqlx"
	_ "github.com/lib/pq"
)

func main() {
	dsn := "host=localhost dbname=postgres"
	db := sqlx.MustConnect("postgres", dsn)

	db.MustExec(`
DROP TABLE IF EXISTS sqlx_test;
CREATE TABLE sqlx_test ( x text );
INSERT INTO sqlx_test (x) VALUES ('a'), ('b');
`)

	var s []string
	if err := db.Select(&s, "SELECT x FROM sqlx_test"); err != nil {
		log.Fatal(err)
	}
	if err := db.Select(&s, "SELECT x FROM sqlx_test"); err != nil {
		log.Fatal(err)
	}
	fmt.Println(s) // prints [a b a b]

	if err := json.Unmarshal([]byte(`["json"]`), &s); err != nil {
		log.Fatal(err)
	}
	fmt.Println(s) // prints [json]
}

The behavior is a little surprising, and it's also the opposite of how JSON unmarshaling behaves:

To unmarshal a JSON array into a slice, Unmarshal resets the slice length to zero and then appends each element to the slice.

Below I'll describe why this causes problems for us.

I propose that we change the behavior of Select to first reset the slice length, just like json.Unmarshal.

It seems possible that people depend on this behavior due to Hyrum's Law, so if you want to avoid breaking them, I guess a slightly different change would be needed. Perhaps a DB-level bool option?

If there's a change you're willing to accept here, we can send a PR.

---

Here's where this causes problems for us.

We have some applications that make heavy use of PostgreSQL using the serializable isolation level. In this mode, all queries (including read queries) need to be retried on serialization failures. The way this looks is that we have some helper functions which take a querying callback and run it inside a retry loop. Here's a slightly simplified example:

func QueryWithTxRetries(db *sqlx.DB, fn func(*sqlx.Tx) error) error {
	tryTxFn := func() error {
		tx, err := db.Beginx()
		if err != nil {
			return err
		}
		defer tx.Rollback()
		if err := fn(tx); err != nil {
			return err
		}
		return tx.Commit()
	}

	for {
		if err := err = tryTxFn(); err == nil || !IsSerializationFailure(err) {
			return err
		}
	}
}

// ...

var customers []string
err := QueryWithTxRetries(db, func(tx *sqlx.Tx) error {
	customers = customers[:0] // important but easy to miss!
	if err := tx.Select(&customers, "SELECT customer FROM ..."); err != nil {
		return err
	}
})

If your callback happens to use Select, it needs to be careful to clear the slice first. Otherwise when the retry loop runs twice, you may get extra data in your output slice.

This bug is incredibly easy to introduce, and it's also easy to miss, since most of the time the retry loop only runs once.

[cespare]

Fixed by #767.

[allesan]

This change has broken my application.