woylie / flop

Filtering, ordering and pagination for Ecto
MIT License
636 stars 35 forks source link

[Bug] :empty/:not_empty filters not working for SQLite #446

Open martosaur opened 8 months ago

martosaur commented 8 months ago

Summary

:empty and :not_empty filters doesn't seem to be working for SQLite DB

Steps to reproduce

Setup flop for a project with SQLite DB and try to filter a column using :empty op.

Expected behaviour

Columns is filtered normally

Actual behaviour

Query always returns empty list

Elixir/Erlang version

Elixir 1.15.7 (compiled with Erlang/OTP 26) Elixir 1.16.0 (compiled with Erlang/OTP 26)

Flop and Ecto versions

* ecto 3.11.1 (Hex package) (mix)
* ecto_sql 3.11.1 (Hex package) (mix)
  locked at 3.11.1 (ecto_sql) ce14063a
* ecto_sqlite3 0.14.0 (Hex package) (mix)
  locked at 0.14.0 (ecto_sqlite3) 4f69e5df
* flop 0.24.1 (Hex package) (mix)
  locked at 0.24.1 (flop) aecbacdf
* flop_phoenix 0.22.5 (Hex package) (mix)
  locked at 0.22.5 (flop_phoenix) 6dec7e21

The test is written for main's head

Additional context

I was able to dig it to this point. Consider this test:

    test "applies empty filter" do
      require Flop.Adapter.Ecto.Operators

      field = :species

      d1 = dynamic([r], is_nil(field(r, ^field)) == ^true); d2 = dynamic([r], Flop.Adapter.Ecto.Operators.empty(:other) == ^true)

      assert where(Pet, ^d1) == where(Pet, ^d2)
    end

It would be reasonable to expect those queries to be equal, but they are different:

     left:  %Ecto.Query{
              ...
              wheres: [
                %Ecto.Query.BooleanExpr{
                  ...
                  params: [true: :boolean]
                }
              ]
            }
     right: %Ecto.Query{
              ...
              wheres: [
                  ...
                  params: [true: :any]
                }
              ]
            }

I suspect SQLite adapter might have a different behaviour for true: :any param then postgres, hence the issue.

woylie commented 8 months ago

Thank you for the report. We should probably add more test repos for other adapters than postgrex and automatically run the existing tests on all of those repos. I don't know when I'll get to setting this up, though. Contributions are welcome.