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

add eqNullable #463

Closed
wants to merge 1 commit into from
Closed

add eqNullable #463

wants to merge 1 commit into from

Conversation

wyozi
Copy link
Contributor

@wyozi wyozi commented Aug 22, 2023

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
Copy link
Member

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 👍

@soedirgo soedirgo closed this Aug 24, 2023
@wyozi wyozi deleted the nullable-eq branch August 24, 2023 07:49
@wyozi
Copy link
Contributor Author

wyozi commented Aug 28, 2023

@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
Copy link
Member

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

@steve-chavez
Copy link
Member

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

@soedirgo
Copy link
Member

soedirgo commented Aug 30, 2023

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
Copy link
Member

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
Copy link
Member

OK - created an issue to warn users about this behavior: #473

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

Successfully merging this pull request may close these issues.

3 participants