`SELECT ... WHERE col = ($foo[:])` behaves inconsistantly based on the length of foo
- Dominant language
- Go
- Stars
- 19
- Forks
- 10
- PR merge metrics
- No merged PRs in 30d
Description
`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
```golang
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`
Contributor guide
Research direction
Start by running the provided Go reproduction and compare TestTestSingular with TestTestMultiple, then trace how the SQL query expands ($ids[:]) before execution. Identify the query-processing entry point responsible for the differing behavior and add regression coverage for both one-value and multiple-value inputs. Done means the two cases behave consistently without the current silent false positive or row-value error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql, sqlite
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100