supabase / postgrest-js

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

Database typings are not correct on table join for entity to one relationship #408

Open niels-bosman opened 1 year ago

niels-bosman commented 1 year ago

Bug report

Describe the bug

Typings are not correct when joining tables that are always NOT an array. For example, in my application we use the following query to fetch data about figma_comments and link to figma_users and figma_files:

const { data: comments } = await supabase
    .from('figma_comments')
    .select(
      'created_at, resolved_at, comment, node_id, user:figma_users(handle, img_url, email), file:figma_files(file_key, name)',
    )
    .order('created_at', { ascending: false });

From this query, I would expect the built in typings to have both user and file to be an entity of a singular user and a singular file. Since the constraints I set up in Postgres won’t allow it to be multiple entries because the foreign key is unique on the joinable tables.

Here is an overview of the types being generated by supabase CLI:

Tables: {
      figma_comments: {
        Row: {
          comment: string;
          created_at: string;
          figma_user_id: string;
          file_key: string;
          id: number;
          node_id: string;
          order_id: number | null;
          parent_id: string | null;
          resolved_at: string | null;
        };
        Insert: {
          comment: string;
          created_at?: string;
          figma_user_id: string;
          file_key: string;
          id?: number;
          node_id: string;
          order_id?: number | null;
          parent_id?: string | null;
          resolved_at?: string | null;
        };
        Update: {
          comment?: string;
          created_at?: string;
          figma_user_id?: string;
          file_key?: string;
          id?: number;
          node_id?: string;
          order_id?: number | null;
          parent_id?: string | null;
          resolved_at?: string | null;
        };
      };
      figma_files: {
        Row: {
          editor_type: string | null;
          figma_project_id: number | null;
          file_key: string;
          id: number;
          last_modified: string;
          name: string;
          thumbnail_url: string;
          version: string | null;
        };
        Insert: {
          editor_type?: string | null;
          figma_project_id?: number | null;
          file_key: string;
          id?: number;
          last_modified: string;
          name: string;
          thumbnail_url: string;
          version?: string | null;
        };
        Update: {
          editor_type?: string | null;
          figma_project_id?: number | null;
          file_key?: string;
          id?: number;
          last_modified?: string;
          name?: string;
          thumbnail_url?: string;
          version?: string | null;
        };
      };
      figma_users: {
        Row: {
          email: string;
          figma_id: string;
          handle: string;
          id: number;
          img_url: string;
        };
        Insert: {
          email: string;
          figma_id: string;
          handle: string;
          id?: number;
          img_url: string;
        };
        Update: {
          email?: string;
          figma_id?: string;
          handle?: string;
          id?: number;
          img_url?: string;
        };
      };
    };

Expected behavior

I would expect the built in typings to know that user and file in the case of my query is an object if the entity, not an array of objects. This is because figma_files.file_key and figma_users.figma_id are set to UNIQUE.

I understand that making this work might be extremely difficult, but does anyone have any suggestions on how to make the typings work for me so I do not have to cast these types?

System information

selbyk commented 1 year ago

:+1:

And also looking for an elegant way to deal with all the {} | {}[] | null in the types.

niels-bosman commented 1 year ago

Hey @selbyk, I managed to fix it for now by chaining with the .returns<T>(); method. This will overwrite the type that is returned (so it is not really that safe).

type Tables = IDatabase['public']['Tables'];
type TeamsTable = Tables['figma_teams']['Row'];
type FigmaUsersTable = Tables['figma_users']['Row'];

export type Team = {
  team: {
    id: TeamsTable['id'];
    name: TeamsTable['name'];
    projects: [
      {
        files: [
          {
            comments: [
              {
                member: { img_url: FigmaUsersTable['img_url'] };
              },
            ];
          },
        ];
      },
    ];
  };
};

const { data, error } = await (client ?? supabase)
    .from('user_has_figma_team')
    .select(
      `team:figma_teams(
         id,
         name,
         projects:figma_projects(
           files:figma_files(
             comments:figma_comments(
               member:figma_users(img_url)
             )
           )
         )
      )`,
    )
    .eq('user_id', user?.id)
    .returns<Team[]>()
    .order('created_at', { foreignTable: 'figma_teams', ascending: false });
imownbey commented 1 year ago

I am seeing the return types be correct as the #223 states, but the typescript types are still totally ambiguous between array or single response which makes the return types somehow even less useful? Is there a way to get the types to match the return type?

imownbey commented 1 year ago

Ah I see #303 is really where the discussion on this is happening.

arinmirza commented 10 months ago

Ah I see #303 is really where the discussion on this is happening.

That issue was recently closed in favor of this one.

👍

And also looking for an elegant way to deal with all the {} | {}[] | null in the types.

Any updates on this?

Apart from the original problem described in this issue, selecting from a foreign table returns the type {} | {}[] | null for that field, although it must be strictly {}[].

Consider the scenario where we have two tables called students and books, and we have 1:N relationship between these two. The books table has a foreign key referring to the primary key of the students table. So we expect to see multiple books associated with a single student.

const { data } = await supabase.from("students").select("*, books (*)");

Can I safely assume that these points hold true?

If so, I will use a workaround and override these types, until a fix for this problem is implemented.

albertonii commented 5 months ago

Any updated with this issue?

albertonii commented 5 months ago

I found temporary fix in this discussion comment:

https://github.com/orgs/supabase/discussions/7610#discussioncomment-7517390

logemann commented 4 months ago

This workaround doesnt change anything for me. Weird. Still a super annoying bug which makes using types with supabase nearly impossible. This bug needs more love from the supabase team. because you know.... 1:1 relations are not that uncommon to say the least.

In case the supabase team is looking into this, i ve mentioned the exact same problem in Discord. I replay here:


somehow the supabase types generator, generates 1:n types where my DB design has a 1:1 mapping.

Following table structure

Person                      Address
------                      -------
id                          id (PK)
name                        street
address_id (FK)
const myQuery = supabase
    .from("Person")
    .select("name, Address(street)");

type PersonWithAddress = QueryData<typeof myQuery>;
const { data: persons, error} = await myQuery;
const personsTyped: PersonWithAddress = persons;

This is basically the example as shown in the supabase docs. The correct results looks like this when debugging:

{
  id: 1,
  name: "Peter",
  Address: { id: 1, street: "MyStreet" }
}

Now the weird part. The type PersonWithAddress looks like this:

const personsTyped: {id: any, name: any, Address: {id: any, street: any}[]}[]

Look at the array just behind the "Address" type (not the outer array, which is the result set array). Address cant be an array because its a 1:1 relation. This means, when mapping through the array like this:

{personsTyped!.map((person) => (
    <div>{person.Address.id}</div>
)}

is wrong according to typescript linter because he expects: person.Address[0].id. During runtime of course, i can only do person.Address.id to get data.

When looking at my generated typescript file:

Relationships: [
          {
            foreignKeyName: "Person_Address_fkey"
            columns: ["address_id"]
            isOneToOne: false
            referencedRelation: "Address"
            referencedColumns: ["id"]
          },

Why is "OneToOne" false ?? The address_id (FK field) is a number column. This definitely should be true right? Even if i set it to true (manually, which is.a non starter anyhow), it doesnt change the outcome of the linter.

hawkcookie commented 4 months ago

This workaround doesnt change anything for me. Weird. Still a super annoying bug which makes using types with supabase nearly impossible. This bug needs more love from the supabase team. because you know.... 1:1 relations are not that uncommon to say the least.

In case the supabase team is looking into this, i ve mentioned the exact same problem in Discord. I replay here:

somehow the supabase types generator, generates 1:n types where my DB design has a 1:1 mapping.

Following table structure

Person                      Address
------                      -------
id                          id (PK)
name                        street
address_id (FK)
const myQuery = supabase
    .from("Person")
    .select("name, Address(street)");

type PersonWithAddress = QueryData<typeof myQuery>;
const { data: persons, error} = await myQuery;
const personsTyped: PersonWithAddress = persons;

This is basically the example as shown in the supabase docs. The correct results looks like this when debugging:

{
  id: 1,
  name: "Peter",
  Address: { id: 1, street: "MyStreet" }
}

Now the weird part. The type PersonWithAddress looks like this:

const personsTyped: {id: any, name: any, Address: {id: any, street: any}[]}[]

Look at the array just behind the "Address" type (not the outer array, which is the result set array). Address cant be an array because its a 1:1 relation. This means, when mapping through the array like this:

{personsTyped!.map((person) => (
    <div>{person.Address.id}</div>
)}

is wrong according to typescript linter because he expects: person.Address[0].id. During runtime of course, i can only do person.Address.id to get data.

When looking at my generated typescript file:

Relationships: [
          {
            foreignKeyName: "Person_Address_fkey"
            columns: ["address_id"]
            isOneToOne: false
            referencedRelation: "Address"
            referencedColumns: ["id"]
          },

Why is "OneToOne" false ?? The address_id (FK field) is a number column. This definitely should be true right? Even if i set it to true (manually, which is.a non starter anyhow), it doesnt change the outcome of the linter.

@logemann

What version of supabase-js are you using? There was a bug previously, but it was fixed in postgrest-js v1.9.0. Then, postgrest-js v1.9.0 was incorporated into the release of supabase-js v2.39.2.

https://github.com/supabase/postgrest-js/releases/tag/v1.9.0 https://github.com/supabase/supabase-js/releases/tag/v2.39.2

And, how are you generating the schema through supabase cli? There was such an exchange in this issue.

https://github.com/supabase/postgrest-js/issues/303#issuecomment-1564203029

I'm doing fine with the following cli command: supabase gen types typescript --local > schema.ts

plz also check supabse cli version.

logemann commented 4 months ago

oh you are right! I was on SDK 2.39.0 ... so slightly before the fix. Damn ;-) I should have upgraded before asking. Thanks for pointing out.

I tested my code again and now all works as expected.

ardabeyazoglu commented 2 months ago

I updated supabase-cli and supabase-js to latest versions and the problem still persists. Generated type relation is still not one-to-one (supabase gen types typescript --linked --schema public > schema.ts).

Ekran görüntüsü 2024-04-24 120126

hawkcookie commented 2 months ago

I updated supabase-cli and supabase-js to latest versions and the problem still persists. Generated type relation is still not one-to-one (supabase gen types typescript --linked --schema public > schema.ts).

Ekran görüntüsü 2024-04-24 120126

I haven't checked what it's like now, but to correctly generate types, please try using the --local option.

https://github.com/supabase/postgrest-js/issues/303#issuecomment-1564203029

oussamabenkortbi commented 2 months ago

Same here, type error when two level of joins! // Property 'last_name' does not exist on type 'string & {}[]'.ts(2339)

XxStunner commented 1 month ago

Having the same issue here.

niels-bosman commented 1 month ago

I don’t know why people are not talking about this. This was single handedly the reason why using Supabase was not feasible for our team..

jdgamble555 commented 1 month ago

@soedirgo - You can probably close this one as related to #536 considering there is a work around now.

ardabeyazoglu commented 1 month ago

I updated supabase-cli and supabase-js to latest versions and the problem still persists. Generated type relation is still not one-to-one (supabase gen types typescript --linked --schema public > schema.ts). Ekran görüntüsü 2024-04-24 120126

I haven't checked what it's like now, but to correctly generate types, please try using the --local option.

#303 (comment)

Doesnt matter, it is the same.

jdgamble555 commented 1 month ago

I updated supabase-cli and supabase-js to latest versions and the problem still persists. Generated type relation is still not one-to-one (supabase gen types typescript --linked --schema public > schema.ts). Ekran görüntüsü 2024-04-24 120126

I haven't checked what it's like now, but to correctly generate types, please try using the --local option. #303 (comment)

Doesnt matter, it is the same.

What query and data model are you using? Did you check out my post #536 ?

J

ardabeyazoglu commented 1 month ago

@jdgamble555 I checked that issue and tried the recommended method with no luck.

Using city:cities!city_id(name) results as following:

(property) city: {
    name: any;
}[]

I have relation like this in generated schema.ts, for account table.

Relationships: [
          {
            foreignKeyName: "public_accounts_city_id_fkey"
            columns: ["city_id"]
            isOneToOne: true
            referencedRelation: "cities"
            referencedColumns: ["id"]
          }
]

EDIT: Might be an issue with my ide or compiler, i ll start from scratch and try again first.

jdgamble555 commented 1 month ago

@ardabeyazoglu - Did you try?

city:cities!inner(name)

J

ardabeyazoglu commented 1 month ago

@jdgamble555 finally it worked with using !inner, after regenerating types and creating client with createClient.

I think the issue can be closed.

fvermaut commented 3 weeks ago

I tried everything in this thread, as well as in other threads, but I still have the issue. Essentially, for 1-to-1 relationships, the typing expects a T[] for the nested data, while the data effectively returned is a single T.

I updated both the CLI and supabase-js to latest versions, re-generated the types, but no luck. I must admit this is quite annoying, and i's not clear to me if the issue is fixed, or if there is workaround, and in which case what the workaround is ...

jdgamble555 commented 3 weeks ago

@fvermaut - What is your data model and query?

fvermaut commented 3 weeks ago

Well essentially it happens between those 2 tables:

CREATE TABLE IF NOT EXISTS "public"."user_projects" (
    "id" "uuid" DEFAULT "gen_random_uuid"() NOT NULL,
    "created_at" timestamp with time zone DEFAULT "now"() NOT NULL,
    "updated_at" timestamp with time zone DEFAULT "now"(),
    "name" "text",
    "req_status" "public"."project_status" DEFAULT 'CREATED'::"public"."project_status",
    "req_hyperparams_map" "json",
    "req_start_date" "date",
    "req_settings" "json",
    "req_type" "public"."project_types" DEFAULT 'LIVE'::"public"."project_types" NOT NULL,
    "req_model_id" "uuid",
    "user_id" "uuid" DEFAULT "auth"."uid"(),
    "req_source_id" "uuid"
CREATE TABLE IF NOT EXISTS "public"."projects" (
    "id" "uuid" NOT NULL,
    "created_at" timestamp with time zone DEFAULT "now"() NOT NULL,
    "updated_at" timestamp with time zone DEFAULT "now"(),
    "name" "text",
    "hyperparams_map" "json",
    "evaluation_criterias" "json",
    "type" "public"."project_types" DEFAULT 'LIVE'::"public"."project_types",
    "start_date" "date",
    "last_executed_at" timestamp with time zone,
    "status" "public"."project_status" DEFAULT 'INIT'::"public"."project_status" NOT NULL,
    "target_status" "public"."project_status" DEFAULT 'INIT'::"public"."project_status" NOT NULL,
    "error_message" "text",
    "settings" "json",
    "source_id" "uuid"
);

Note that the "id" of the second table is both a primary key, and a foreign key to the first one:

ALTER TABLE ONLY "public"."projects"
    ADD CONSTRAINT "project_pkey" PRIMARY KEY ("id");
ALTER TABLE ONLY "public"."projects"
    ADD CONSTRAINT "public_projects_id_fkey" FOREIGN KEY ("id") REFERENCES "public"."user_projects"("id") ON UPDATE CASCADE ON DELETE CASCADE;

My debug query is this one:

const supabase = createClient();

const respProjects = await supabase
    .from("user_projects")
    .select("id, project:projects!inner(*)");

if (!respProjects.error) {
    const projects = respProjects.data;
    console.log(projects[0].project.name);

What is the problem: the "name" field is appearing red in VScode, even if it is running fine - because typing expects an array there. The only way to remove the typescript error is ito replace it with 'projects[0].project[0].name', which though doesn't work at runtime.

On a side note, the logic behind my model (but should not be relevant to current issue): I want to give WRITE access to authenticated users( through RLS) only to the "users_projects" table - and I have a custom backend process using the service key, that manages the update of the "projects" table based on the requested status ("req_status") set on the "user_projects" row.

jdgamble555 commented 3 weeks ago

@fvermaut - I'm not having that problem. I simplified your schema to:

CREATE TABLE IF NOT EXISTS "public"."user_projects"(
    "id" "uuid" PRIMARY KEY DEFAULT "gen_random_uuid"() NOT NULL,
    "created_at" timestamp with time zone DEFAULT "now"() NOT NULL,
    "updated_at" timestamp with time zone DEFAULT "now"(),
    "name" "text",
    "req_hyperparams_map" "json",
    "req_start_date" "date",
    "req_settings" "json",
    "req_model_id" "uuid",
    "user_id" "uuid" DEFAULT "auth"."uid"(),
    "req_source_id" "uuid"
);

CREATE TABLE IF NOT EXISTS "public"."projects"(
    "id" "uuid" NOT NULL PRIMARY KEY REFERENCES public.user_projects(id),
    "created_at" timestamp with time zone DEFAULT "now"() NOT NULL,
    "updated_at" timestamp with time zone DEFAULT "now"(),
    "name" "text",
    "hyperparams_map" "json",
    "evaluation_criterias" "json",
    "start_date" "date",
    "last_executed_at" timestamp with time zone,
    "error_message" "text",
    "settings" "json",
    "source_id" "uuid"
);

Here is the types you get for:

await supabase.from('user_projects').select("id, project:projects!inner(*)")
PostgrestSingleResponse<{
    id: string;
    project: {
        created_at: string;
        error_message: string | null;
        evaluation_criterias: Json;
        hyperparams_map: Json;
        id: string;
        last_executed_at: string | null;
        ... 4 more ...;
        updated_at: string | null;
    } | null;
}[]>

The reason it is possibly null is because user_projects don't have an FK that mandates it exists. If you put the FK the other way around, you won't that problem. Either way, I'm not getting an array as an option.

https://github.com/jdgamble555/supabase-types/blob/master/src/lib/index.ts

J

fvermaut commented 3 weeks ago

Thanks for checking this @jdgamble555 - still I tried to modify the schema as you did (declare the pk and fk directly in the table) and I still get an array. Question: you're not having the problem because you changed the schema? Also, I'm not sure to understand the test you did here:

await supabase.from('posts').select('*, user:users!inner(*)');

as I don't have these tables in my schema. Can you share your code and how you get that output, so that I can compare?

jdgamble555 commented 3 weeks ago

I pasted the wrong query. Fixed now.

https://github.com/jdgamble555/supabase-types/blob/master/supabase/migrations/20240520210322_initial.sql

J

fvermaut commented 3 weeks ago

@jdgamble555 I appreciate your tentative to help, but still the issue persists for me, even though I used the latest versions, and tried the same schema as you mentioned. It's also not clear why it works for you (on your side, does it work because you simplified the schema as I asked I previous comment?), and not for me. In any cases, the schema I use is the one generated by Supabase, so in any case there would be DX improvement to be made here...

Anyway, I decided to cut the problem and not lose any more time on this, by not making the join and directly querying the second table (as fortunately I found a way to avoid it).

I still believe there is probably something to be fixed, as this is not the only issue reported. Here are 2 other open ones which describe exactly the same problem as I have:

jdgamble555 commented 3 weeks ago

@fvermaut - The only solution to get the right code is to use !inner as I showed. This is not a work around, but the solution. Technically, the problem going from the other direction is a different problem. We don't want to leave five of these posts open, as it will never get solved the right way.

Let's diagnose why yours won't work. You can clearly spin up my Supabase instance above and it works, so your schema has to be significantly different in the PKs and / or the FKs.

What happens when you run this query after regenerating the database.d.ts file?

await supabase.from('user_projects').select("id, project:projects!inner(*)")

What is the result?

Is the id field in public.user_projects a PK? You didn't state this, but I assumed it. You also didn't finish posting the whole schema, so I'm assuming there are no other fields.

Something is different, as you shouldn't have this problem. Please give more information.

Thanks,

J

bnjmnt4n commented 3 weeks ago

@fvermaut - The only solution to get the right code is to use !inner as I showed. This is not a work around, but the solution.

To me, this is definitely a workaround. There's no reason why you'd have to change your query to get correct types.

jdgamble555 commented 3 weeks ago

@bnjmnt4n - Technically, the only types that do not work correctly are:

Querying embedded tables through columns (user:user_id(*)) and using fkey columns as hints

from #536.

Using inner for that use case is definitely a work-around. However, @fvermaut is not getting the work-around to work. My point is that we need to stick to the issue at hand as these posts get overwhelmed with 50 different issues and comments, which sometimes are inundated with incorrect information.

If we don't stick to the exact problem, it will never get fixed.

@fvermaut's code is all over the place, for example, so I suspect something else is going on with his code. If there is a bug with the work-around, we need to know.

J

fvermaut commented 3 weeks ago

Sorry, again as I said I won't spend more time to debug this issue, as unfortunately I simply don't have time for this. If the only solution is a workaround, then I found a simpler workaround by just skipping the join. On a side note, and IMHO: typing should be always consistent with the data returned from the API. You shouldn't have to write your queries in a particular way to make typing work. Typing is supposed to validate the queries, not the other way around. If you have to write your queries in a particular way to make typing consistent with the results of the query, then what's the point of using typing in the first place?
And also, why do you say my code is not "all over the place"?? the SQL I shared is all generated by Supabase Studio, and the JS is the same as the one you shared.

jdgamble555 commented 3 weeks ago

@fvermaut - Just trying to solve the problem. 😄 You pasted the generated code from postgres instead of your schema directly, you left out some things, and I still don't know the PK situation. More than one thing is missing and not clear. FWI, this is the advantage of developing locally first.

The reason you want to debug this is so you (and the benefit of those on this post) can have a better understanding of how to do it correctly. Maybe this time you can avoid the join, but you inevitably will have to do an inner join like that in the future, so you should understand how to do it (even with the work-around) to appease the Type system.

100% we need the Types to work when querying a FK directly, but that is a very specific issue. It does not mean the whole system is broken. People will find this post (and others) and assume that is the case, and nothing gets solved.

If you're not sticking with Supabase, the time involved would not be worth it. If you are, I think we can debug it.

J

fvermaut commented 3 weeks ago

I agree that to get down to the issue you have to investigate every cases, I'm just saying that I don't have time for this, especially if the point is to validate a workaround. I use (and pay for) Supabase as a BaaS, on the premise that you can "build in a weekend" (I know it's a marketing stunt, but still the idea is there). So by principle I don't want to spend weekends debugging things like this. I'll keep watching the thread if something clicks at some point. Thanks again for your help.

tillka commented 2 weeks ago

@fvermaut - The only solution to get the right code is to use !inner as I showed. This is not a work around, but the solution. Technically, the problem going from the other direction is a different problem. We don't want to leave five of these posts open, as it will never get solved the right way.

Let's diagnose why yours won't work. You can clearly spin up my Supabase instance above and it works, so your schema has to be significantly different in the PKs and / or the FKs.

What happens when you run this query after regenerating the database.d.ts file?

await supabase.from('user_projects').select("id, project:projects!inner(*)")

What is the result?

Is the id field in public.user_projects a PK? You didn't state this, but I assumed it. You also didn't finish posting the whole schema, so I'm assuming there are no other fields.

Something is different, as you shouldn't have this problem. Please give more information.

Thanks,

J

I came to this thread after googling my supabase error.

!inner works fine if you have only one reference from one table to another but breaks if you have two references in one table.

E.g. my listings have two references to profiles, one for the creator_id and one for owner_id. In this case a owner:profiles!owner_id(*) actually pulls the right data (a single profile) but provides the wrong type {}[]. Using owner:profiles!inner(*) provides the correct type (a single profile) but throws an error: "Could not embed because more than one relationship was found for 'listings' and 'profiles'". Something is off.

Is there a way to further disambiguate the inner join? I would expect either owner:profiles!inner!owner_id(*) or owner:profiles!owner_id!inner(*) to work and provide the corrext type.

jdgamble555 commented 2 weeks ago

@tillka - Yeah that makes sense. There would be no way for the query to know the name of the column coming from the other direction. This is an impossible type scenario.

@soedirgo - This is an example of an impossible situation, and why the types need to work coming from BOTH directions.

J

tillka commented 1 week ago

@tillka - Yeah that makes sense. There would be no way for the query to know the name of the column coming from the other direction. This is an impossible type scenario.

Thought so, but shouldn't this work (correctly) then?

owner:profiles!owner_id(*) actually pulls the right data (a single profile) but provides the wrong type {}[]

jdgamble555 commented 1 week ago

Yes, it should, but Supabase currently doesn't support typing from a foreign key field.

I guess they figured there is a work around so they haven't prioritized it, but now we know one at least one situation without a work-around.

J