Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change in behaviour of bun.In with empty slice #902

Open
rockwurst opened this issue Sep 19, 2023 · 3 comments
Open

Change in behaviour of bun.In with empty slice #902

rockwurst opened this issue Sep 19, 2023 · 3 comments

Comments

@rockwurst
Copy link

Problem

Consider a query using bun.In with an empty slice:

db.NewSelect().Where("id NOT IN (?)", bun.In([]int{}))

In bun v1.1.12 it produces this SQL for all dialects:

SELECT * WHERE (id NOT IN ())

In bun v1.1.13 and later releases, it produces this SQL for all dialects:

SELECT * WHERE (id NOT IN (NULL))

sqlite is the only supported dialect that would accept the id NOT IN () syntax. Changing it to id NOT IN (NULL) in v1.1.13 means the behaviour has changed from matching all rows to matching no rows. Test using sqlite3 command line interface:

sqlite> CREATE TABLE test ("id" int PRIMARY KEY);
sqlite> INSERT INTO test VALUES (1),(2),(3);
sqlite> SELECT * FROM test WHERE id NOT IN ();
1
2
3
sqlite> SELECT * FROM test WHERE id NOT IN (NULL);
sqlite>

For all other dialects, v1.1.12's id NOT IN () is not valid syntax. e.g. postgres will error with:

ERROR:  syntax error at or near ")" (SQLSTATE=42601)

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:

Note that SQLite allows the parenthesized list of scalar values on the right-hand side of an IN or NOT IN operator to be an empty list but most other SQL database engines and the SQL92 standard require the list to contain at least one element.

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.

@rockwurst
Copy link
Author

Looks like this bug was introduced by 1e624cc. See the change in schema/append.go

@rockwurst
Copy link
Author

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))

@jakeczyz
Copy link

This also affected me. Any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants