supabase / postgrest-js

Isomorphic JavaScript client for PostgREST.
https://supabase.com
MIT License
964 stars 128 forks source link

add eqNullable #463

Closed wyozi closed 10 months ago

wyozi commented 10 months ago

What kind of change does this PR introduce?

Add a new filter eqNullable that uses either .eq or .is depending on whether the input parameter is null

Why

Currently, using .eq() with a parameter that is allowed to be null is extremely unergonomic. You'll have to do something like

      if (service_recipient_id) {
        query = query.eq(
          "service_recipient_id",
          service_recipient_id
        );
      } else {
        query = query.is(
          "service_recipient_id",
          null
        );
      }

This method simplifies that to query.eqNullable("id", id) and allows chaining it as normal.

soedirgo commented 10 months ago

Thanks @wyozi! I'll close this in favor of your other PR - in supabase-js v3 we'll merge .eq() and .is() so there's no confusion on this :+1:

wyozi commented 10 months ago

@soedirgo I was just bit hard by neq not matching null values (i.e. .neq("col", "foo") did not find columns where col = NULL).

Do you reckon something similar would be good for .neq() as well, so .neq("col", "foo") would expand to .or("(col.is.null,col.neq.foo)") in practice?

soedirgo commented 10 months ago

@steve-chavez wdyt? This is kind of a SQL wart, not sure if we should make the behavior more intuitive here.

steve-chavez commented 10 months ago

@soedirgo Merging eq() with is() client-side seems good to me.

soedirgo commented 10 months ago

What about .neq()? I don't think we'll need it for other filters, so it's just .eq() and .neq(), but making .neq() translate to or(col.is.null,col.neq.foo) - while more intuitive - deviates from SQL behavior.

steve-chavez commented 10 months ago

making .neq() translate to or(col.is.null,col.neq.foo)

That might give a surprising query plan, I don't think we should do it.

soedirgo commented 10 months ago

OK - created an issue to warn users about this behavior: https://github.com/supabase/postgrest-js/issues/473