`SELECT ... WHERE col = ($foo[:])` behaves inconsistantly based on the length of foo

[jack-w-shaw]

SELECT ... WHERE col = ($foo[:]) has a nasty gotcha.

If the query happens to use a foo of length 1, the query evalutes nicely to WHERE col = val.

However, if foo contains multiple values, we (correctly imo) error out with row value misued.

This is a nasty gotcha because in test cases it is not at all unusual to test with only one value in foo, which will result in a silent false positive. So it's very possible that unit tests, integration tests, and manual tests will miss this failure under certain circumstances

Also, SELECT ... WHERE col = ($foo[:]) and the correct syntax SELECT ... WHERE col IN ($foo[:]) are very similar, it's very likely a reviewer will miss this error

Replication steps

package juju

import (
	"context"
	"testing"

	"github.com/canonical/sqlair"
	"github.com/juju/tc"

	schematesting "github.com/juju/juju/domain/schema/testing"
)

type suite struct {
	schematesting.ModelSuite
}

func TestSuite(t *testing.T) {
	tc.Run(t, &suite{})
}

type (
	life struct {
		ID    int    `db:"id"`
		Value string `db:"value"`
	}
	ids []int
)

var stmt = sqlair.MustPrepare(`
SELECT &life.*
FROM life
WHERE id = ($ids[:])
	`, life{}, ids{})

func (s *suite) TestTestSingular(c *tc.C) {
	lifeIDs := ids([]int{0})
	err := s.TxnRunner().Txn(c.Context(), func(ctx context.Context, tx *sqlair.TX) error {
		return tx.Query(ctx, stmt, lifeIDs).GetAll(&[]life{})
	})
	c.Assert(err, tc.ErrorIsNil)
}

func (s *suite) TestTestMultiple(c *tc.C) {
	lifeIDs := ids([]int{0, 1})
	err := s.TxnRunner().Txn(c.Context(), func(ctx context.Context, tx *sqlair.TX) error {
		return tx.Query(ctx, stmt, lifeIDs).GetAll(&[]life{})
	})
	c.Assert(err, tc.ErrorIsNil)
}

Results in:

$ go test ./
--- FAIL: TestSuite (0.12s)
    --- FAIL: TestSuite/TestTestMultiple (0.07s)
        /home/jack/playground/main_test.go:44
        main_test.go:56: 
                c.Assert(err, tc.ErrorIsNil)
            ... value *errors.Err = &errors.unformatter{message:"", cause:sqlite3.Error{Code:1, ExtendedCode:1, SystemErrno:0x0, err:"row value misused"}, previous:(*errors.Err)(0xc0004e1590), function:"github.com/juju/retry.Call", line:188} ("row value misused")
            ... error stack:
            	row value misused
            	github.com/juju/juju/internal/database/testing.(*txnRunner).Txn.func1.(*RetryingTxnRunner).Txn.2:166: 
            	github.com/juju/juju/internal/database/txn.(*RetryingTxnRunner).run:322: 
            	github.com/juju/juju/internal/database/testing.(*txnRunner).Txn.func1:34: 
            	github.com/juju/juju/internal/database/txn.newOptions.DefaultRetryStrategy.func1.1:342: 
            	github.com/juju/retry.Call:188: 
FAIL
FAIL	juju	0.132s
FAIL

Notice that only one of the tests has failed. ad the only difference is the length of lifeIDs

[jack-w-shaw]

Admittedly, it is possible this is just an sql gotcha, in which case it arguably wouldn't be sqlair's responsibility to catch this