Open rockwurst opened 1 year ago
Looks like this bug was introduced by 1e624ccfc966f98ee781c443f13efcb3e3aac12c. See the change in schema/append.go
To reproduce:
git checkout v1.1.12
Add this test case to the end of the queries
declaration in internal/dbtest/query_test.go
:
func(db *bun.DB) schema.QueryAppender {
return db.NewSelect().Where("id IN (?)", bun.In([]int{}))
},
Run the tests (DOCKER_DEFAULT_PLATFORM
required for my M1 Macbook as no arm images exist for the DBs (so without it, docker compose fails with no matching manifest for linux/arm64/v8 in the manifest list entries
))
cd internal/dbtest
DOCKER_DEFAULT_PLATFORM=linux/amd64 ./test.sh
It will fail because we haven't yet created the testdata snapshots for the new test case, but it will create them. e.g.
--- FAIL: TestQuery (0.12s)
--- FAIL: TestQuery/pg (0.02s)
--- FAIL: TestQuery/pg/158 (0.00s)
query_test.go:1006: snapshot created for test TestQuery-pg-158
Inspect what it created for each DB:
$ for f in testdata/snapshots/TestQuery-*-158; do printf '%-45s: %s\n' "$f" "$(cat $f)"; done
testdata/snapshots/TestQuery-mariadb-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-mssql2019-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-mysql5-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-mysql8-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-pg-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-pgx-158 : SELECT * WHERE (id IN ())
testdata/snapshots/TestQuery-sqlite-158 : SELECT * WHERE (id IN ())
Now switch to the v1.1.13 release and rerun the tests:
git checkout v1.1.13
DOCKER_DEFAULT_PLATFORM=linux/amd64 ./test.sh
Observe that the tests fail because the output has changed since v1.1.12:
--- FAIL: TestQuery (0.12s)
--- FAIL: TestQuery/pgx (0.02s)
--- FAIL: TestQuery/pgx/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/mysql5 (0.02s)
--- FAIL: TestQuery/mysql5/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/mysql8 (0.02s)
--- FAIL: TestQuery/mysql8/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/mariadb (0.02s)
--- FAIL: TestQuery/mariadb/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/sqlite (0.01s)
--- FAIL: TestQuery/sqlite/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/mssql2019 (0.02s)
--- FAIL: TestQuery/mssql2019/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
--- FAIL: TestQuery/pg (0.01s)
--- FAIL: TestQuery/pg/158 (0.00s)
query_test.go:1006: snapshot not equal:
--- Previous
+++ Current
@@ -1,2 +1,2 @@
-SELECT * WHERE (id IN ())
+SELECT * WHERE (id IN (NULL))
This also affected me. Any updates?
Problem
Consider a query using
bun.In
with an empty slice:In bun v1.1.12 it produces this SQL for all dialects:
In bun v1.1.13 and later releases, it produces this SQL for all dialects:
sqlite is the only supported dialect that would accept the
id NOT IN ()
syntax. Changing it toid NOT IN (NULL)
in v1.1.13 means the behaviour has changed from matching all rows to matching no rows. Test usingsqlite3
command line interface:For all other dialects, v1.1.12's
id NOT IN ()
is not valid syntax. e.g. postgres will error with:The new v1.1.13 version with the
NULL
is not a syntax error, but is also not the intended behaviour. You would expect a select of values "not in the empty set" to be "all values", but the use of NULL means no rows match.Proposed solution
The sqlite docs say this about the IN and NOT IN operators:
So it would be reasonable for bun to detect an empty slice being passed to
bun.In
and to error I think. That would give consistent behaviour across all dialects. Even though sqlite is capable of coping with the empty set, bun hasn't been generating such since v1.1.12 anyway.