supabase / supabase-js

An isomorphic Javascript client for Supabase. Query your Supabase database, subscribe to realtime events, upload and download files, browse typescript examples, invoke postgres functions via rpc, invoke supabase edge functions, query pgvector.
https://supabase.com
MIT License
3.14k stars 251 forks source link

NEQ doesn't work on joins #1242

Open oddanderson opened 2 months ago

oddanderson commented 2 months ago

Bug report

Describe the bug

Unable to use database filters on inner joins for nullable foreign keys

e.g.

from("tableA").select(", tableB()").eq("tableB.column", value) --> works from("tableA").select(", tableB()").neq("tableB.column", value) --> does not work, always empty

To Reproduce

Schema:

create table "public"."cities" (
    "id" uuid not null default gen_random_uuid(),
    "created_at" timestamp with time zone not null default now(),
    "country_id" uuid not null,
    "name" text not null
);

create table "public"."countries" (
    "id" uuid not null default gen_random_uuid(),
    "created_at" timestamp with time zone not null default now(),
    "active" boolean not null default true,
    "name" text not null,
    "random_id" uuid
);

create table "public"."random" (
    "id" uuid not null default gen_random_uuid(),
    "created_at" timestamp with time zone not null default now()
);

CREATE UNIQUE INDEX cities_pkey ON public.cities USING btree (id);
CREATE UNIQUE INDEX countries_pkey ON public.countries USING btree (id);
CREATE UNIQUE INDEX random_pkey ON public.random USING btree (id);

alter table "public"."cities" add constraint "cities_pkey" PRIMARY KEY using index "cities_pkey";
alter table "public"."countries" add constraint "countries_pkey" PRIMARY KEY using index "countries_pkey";
alter table "public"."random" add constraint "random_pkey" PRIMARY KEY using index "random_pkey";
alter table "public"."cities" add constraint "public_cities_country_id_fkey" FOREIGN KEY (country_id) REFERENCES countries(id) not valid;
alter table "public"."cities" validate constraint "public_cities_country_id_fkey";
alter table "public"."countries" add constraint "public_countries_random_id_fkey" FOREIGN KEY (random_id) REFERENCES random(id) not valid;
alter table "public"."countries" validate constraint "public_countries_random_id_fkey";

Test Code

const client: SupabaseClient = createClient(
    supabaseUrl,
    supabaseKey,
    options,
  );

  const { data: random } = await client.from("random").insert({}).select()
    .single();
  console.log(random);
  const { data: random2 } = await client.from("random").insert({}).select()
    .single();
  const { data: country1 } = await client.from("countries").insert({
    name: "USA",
    random_id: random.id,
  }).select().single();
  const { data: country2 } = await client.from("countries").insert({
    name: "Canada",
    active: false,
  }).select().single();

  const { data: city1 } = await client.from("cities").insert({
    name: "Atlanta",
    country_id: country1.id,
  }).select().single();
  const { data: city2 } = await client.from("cities").insert({
    name: "Seattle",
    country_id: country1.id,
  }).select().single();
  const { data: city3 } = await client.from("cities").insert({
    name: "Toronto",
    country_id: country2.id,
  }).select().single();

  const { data: cities1 } = await client.from("cities").select(
    "id, name, countries!inner(id, name, random_id, active)",
  ).eq("countries.active", true);
  console.log("expect american cities only");
  console.log(cities1);
  const { data: cities2 } = await client.from("cities").select(
    "id, name, countries!inner(id, name, random_id, active)",
  ).eq("countries.active", false);
  console.log("expect canada cities only");
  console.log(cities2);
  const { data: cities3 } = await client.from("cities").select(
    "id, name, countries!inner(id, name, random_id, active)",
  ).neq("countries.random_id", random.id);
  console.log("use neq random, expect canada"); <--- THIS ONE FAILS
  console.log(cities3);
  const { data: cities4 } = await client.from("cities").select(
    "id, name, countries!inner(id, name, random_id, active)",
  ).eq("countries.active", true).eq("countries.random_id", random.id);
  console.log("use random + active, expect usa");
  console.log(cities4);

Output

{
  id: "4efd8ada-5d52-4dfd-9f86-b25768487fbc",
  created_at: "2024-07-05T18:00:19.007316+00:00"
}
expect american cities only
[
  {
    id: "f70133da-114b-4698-b3ba-adefca117ae2",
    name: "Atlanta",
    countries: {
      id: "178cc106-056d-443d-858f-bc3abf57a8d2",
      name: "USA",
      active: true,
      random_id: "4efd8ada-5d52-4dfd-9f86-b25768487fbc"
    }
  },
  {
    id: "a6deb4b5-47df-4de9-b664-6ebcd84fb0ab",
    name: "Seattle",
    countries: {
      id: "178cc106-056d-443d-858f-bc3abf57a8d2",
      name: "USA",
      active: true,
      random_id: "4efd8ada-5d52-4dfd-9f86-b25768487fbc"
    }
  }
]
expect canada cities only
[
  {
    id: "e25dbc47-cd78-4874-8dcc-8409e5dd524b",
    name: "Toronto",
    countries: {
      id: "e3904882-366a-4b47-b60f-c9687592bc08",
      name: "Canada",
      active: false,
      random_id: null
    }
  }
]
use neq random, expect canada cities
[]
use random + active, expect usa cities
[
  {
    id: "f70133da-114b-4698-b3ba-adefca117ae2",
    name: "Atlanta",
    countries: {
      id: "178cc106-056d-443d-858f-bc3abf57a8d2",
      name: "USA",
      active: true,
      random_id: "4efd8ada-5d52-4dfd-9f86-b25768487fbc"
    }
  },
  {
    id: "a6deb4b5-47df-4de9-b664-6ebcd84fb0ab",
    name: "Seattle",
    countries: {
      id: "178cc106-056d-443d-858f-bc3abf57a8d2",
      name: "USA",
      active: true,
      random_id: "4efd8ada-5d52-4dfd-9f86-b25768487fbc"
    }
  }
]

Expected behavior

use neq random, expect canada cities --> should return canadian cities

Screenshots

If applicable, add screenshots to help explain your problem.

System information

Additional context

Add any other context about the problem here.

leohanon commented 1 month ago

This isn't a bug. The issue you're running into here is that you're querying a column that contains NULL values. In databases, a NULL value is an unknown value, not necessarily an "empty" value. In your example, the Toronto record has a random_id of NULL. In other words, you're saying that Toronto has some random_id that you don't know the value of.

When you use .neq("random_id", random.id) you're saying "give me all records that you know for sure have a random_id NOT EQUAL TO random.id".

You expect to get Toronto, because the "random_id" is defined as NULL. BUT since a NULL value is treated as unknown... you can't say for sure that it's not equal to random.id... since it's unknown it technically COULD be equal to random.id. Therefore it doesn't return it as a match.

oddanderson commented 1 month ago

"In databases" should be made more specific to "In Postgres." There are many databases that allow direct comparison operators to null values with the above expected results.

Postgres provides Is distinct from to explicitly handle this case. Is there a reason why the NEQ implementation doesn't use it instead of a != or <>? Are there plans to provide an additional wrapper that uses is distinct from?

It's not ideal to require users to write an RPC or create a custom view in place of using a standard operator, both of which are obfuscated when reading someone else's code. I struggle to imagine a frequent use case of someone using NEQ to a non-null value and expecting to not receive rows with null values, no matter how postgres may have chosen to implement null under-the-hood.

leohanon commented 1 month ago

the NEQ is actually a PostgREST behavior (which is what Supabase uses) https://docs.postgrest.org/en/v12/references/api/tables_views.html

it looks like PostgREST already supports an "isDistinct"... I suspect it should be easy enough for the supabase team to add that in 🤷. They have a separate postgrest-js repo where they have this code... I'm assuming they just need to add it here: https://github.com/supabase/postgrest-js/blob/master/src/PostgrestFilterBuilder.ts