supabase-community / postgrest-csharp

A C# Client library for Postgrest
https://supabase-community.github.io/postgrest-csharp/api/Postgrest.html
MIT License
114 stars 22 forks source link

Calls to `Insert` and `Update` fail when a `Reference` is specified on a Model #81

Closed AdamBuchweitz closed 5 months ago

AdamBuchweitz commented 6 months ago

Bug report

Describe the bug

Trying to insert new rows into a table with a Reference (many-to-one) relationship throws a PostgresException.

PostgrestException: {"code":"PGRST204","details":null,"hint":null,"message":"Column '[table]' of relation '[table]' does not exist"}

To Reproduce

Take the following two simple models:

    [Table("people")]
    public class Person : BaseModel
    {
        [PrimaryKey("id")]
        public string id { get; set; }
        public string first_name { get; set; }
        public string last_name { get; set; }

        [Reference(typeof(Quote))]
        public List<Quote> quotes { get; set; } = new();
    }

    [Table("quotes")]
    public class Quote : BaseModel
    {
        [PrimaryKey("id")]
        public string id { get; set; }
        [Column("quote")]
        public string quote { get; set; }
        [Column("people_id")]
        public string people_id { get; set; }
        [Column("created_at")]
        public DateTime created_at { get; set; }
    }

Now try to insert data:

await supabaseClient.From<Person>().Insert(new Person { first_name = "Leeroy", last_name = "Jenkins"});

Results in: PostgrestException: {"code":"PGRST204","details":null,"hint":null,"message":"Column 'quotes' of relation 'people' does not exist"}

Expected behavior

I would expect a row to be inserted as normal.

System information

Additional context

Without the reference, people data inserts fine! It's only after adding the reference that the exception arises. Interestingly, I can't find any examples of one-to-many relationships here. Perhaps it isn't supported?

Here is the schema:

CREATE TABLE IF NOT EXISTS sandbox.people
(
    id uuid NOT NULL DEFAULT gen_random_uuid(),
    first_name character varying(255) COLLATE pg_catalog."default",
    last_name character varying(255) COLLATE pg_catalog."default",
    CONSTRAINT people_pkey PRIMARY KEY (id)
)

CREATE TABLE IF NOT EXISTS sandbox.quotes
(
    id uuid NOT NULL DEFAULT gen_random_uuid(),
    quote character varying(255) COLLATE pg_catalog."default" NOT NULL,
    created_at timestamp with time zone NOT NULL DEFAULT now(),
    people_id uuid NOT NULL,
    CONSTRAINT quotes_pkey PRIMARY KEY (id),
    CONSTRAINT quotes_people_id_fkey FOREIGN KEY (people_id)
        REFERENCES sandbox.people (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE CASCADE
)
acupofjose commented 6 months ago

@AdamBuchweitz hope you had a nice holiday!

This is currently intended functionality. Or, rather, an expected lack of functionality. But looking at the docs, this isn't documented anywhere as far as I see.

Unfortunately, when the ReferenceAttribute was written inferring nested relationships for inserts and updates was a large undertaking.

The problem being that when performing an insert with related data, the library needs the related keys to perform the inserts (which are usually generated).

So, for your use case, the library would need to translate your model with inserts by creating the Person model first, store the result, and then make separate call(s) to create each of the Quotes using the previously inserted Person model.

It's doable, for sure, but no one has implemented it yet. I'm up to implementing it, but I'm traveling for the next month. So, just being honest, I'll probably be slow implementing it!

Thanks for the issue/enhancement request! Love seeing people using supabase and c#.

AdamBuchweitz commented 5 months ago

Thanks for the reply @acupofjose . Support for inserting/updating referenced data would be really nice, but I'm fine without that. The bug is that the library won't even insert the Person without the referenced data. I have taken to building models with derivatives like this:

    [Table("people")]
    public class Person : BaseModel
    {
        [PrimaryKey("id")]
        public string id { get; set; }
        public string first_name { get; set; }
        public string last_name { get; set; }

        [Reference(typeof(Quote), ReferenceAttribute.JoinType.Left)]
        public List<Quote> quotes { get; set; }

        public PersonInsert ToInsert() => new PersonInsert
        {
            id = id,
            first_name = first_name,
            last_name = last_name,
        };
    }

    [Table("people")]
    public class PersonInsert : Person
    {
        [JsonIgnore]
        public List<Quote> quotes { get; set; }
    }

which allows me to insert a Person with the ToInsert() method:

var person = new Person { first_name = "Leeroy", last_name = "Jenkins"};
await supabaseClient.From<Person>().Insert(person.ToInsert());

Since there is already supposed to be support for ignoreOnInsert and ignoreOnUpdate I thought I should be able to annotate the model with that, but it doesn't help any. I think that's the simplest fix here. What do you think?

acupofjose commented 5 months ago

Ah, yep. I actually noticed that as a bug as I was starting the implementation (it's becoming a rabbit hole lol). But the bug fix for just that is definitely easy enough! Good call.

I'm over here making mountains out of molehills.

Give me a few, I think I can have a patch worked out here for you on this issue.

acupofjose commented 5 months ago

Your request should be supported in v3.4.0! Thanks for the bug report @AdamBuchweitz, apologies for the confusion on my part.

AdamBuchweitz commented 5 months ago

I updated via Nuget, and it works! Thank you so much! Now I feel silly for spending those days hunting this down and working around it.

I hope you don't mind if I throw more reports your way - I'm working on a project and we're using this library in production.

AdamBuchweitz commented 5 months ago

Looks like I got excited and spoke too soon... I didn't realize that the test I ran, switching an argument from new MyModelInsert to new MyModel, just ran ToInsert() on it further down the line.

acupofjose commented 5 months ago

~I updated via Nuget, and it works! Thank you so much! Now I feel silly for spending those days hunting this down and working around it.~

I hope you don't mind if I throw more reports your way - I'm working on a project and we're using this library in production.

Please do, always looking to improve the codebase!

acupofjose commented 5 months ago

Also, to clarify, is this still an issue? Or has it been resolved?

mathewgrabau commented 5 months ago

@acupofjose I think this might be still be going on. It was specifically on the INSERT of a many to many link record that I had the issue. I just upgraded the postgrest-csharp version to 3.4.0 to see if the issue is resolved.

I'm currently attempting to debug the issue using an insert model stand-in for that table. Though, I also noticed that the link table just has the two primary keys.

The exception report is rather vexing as well: {}

The insert data looked correct with model definition that had the three primary keys set on the table model (captured with the debug handler on the Postgrest client):

{"table_1_id":16,"table_y_id":"a85b236b-99f5-4284-8e8a-05703df89534"}

mathewgrabau commented 5 months ago

That may be a false alarm @acupofjose - I will attempt to confirm. I had a typo in my Table attribute (name was misspelled). However, the "NotFound" reason did not propagate to something meaningful. However, I can file another handling or my own PR for that. If I determine this issue's problem is still present, I'll update this ticket.