supabase-community / postgrest-go

Isomorphic Go client for PostgREST. (Now Updating)
https://supabase.io
Apache License 2.0
170 stars 27 forks source link

From() will always append the table name to the baseUrl Path #12

Closed intabulas closed 3 years ago

intabulas commented 3 years ago

Bug report

Describe the bug

When using From(), the code will currently forcibly (+=) add the table name to the client transport baseUrl path. This makes it impossible to reuse a client instance (postgrest.NewClient(<url>L, <schema>, <headers>)) as the second time From() is used the schema becomes <schema>.<table><table>

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

var (
    url = "https://<some-supabase-url>/rest/v1/"
    headers  = map[string]string{"apikey": "<my-supabase-apikey>"}
)

client := postgrest.NewClient(url, "public", headers)

// .. build some list
for _, doc := range listofdocs {
    // Second time this is called, there will be an error because the schema is now wrong
    resp, err := client.From("<mytable>").Insert(doc, "", "", "").Execute()
    if err != nil {
        // handle error
    }
}

Expected behavior

Expected behavior is that if calling multiple times (vs batch insert) that From() wont cause the insert to fail. The JS client (for example) does not exhibit this issue

Screenshots

N/A

Additional context

I am working on a solution to this that I will submit a PR for

darora commented 3 years ago

@intabulas are you planning to make a PR towards this? I took a look at https://github.com/intabulas/postgrest-go/commit/1105e086eb358e9bd311b8e0982c71ed6cd2d9a9 and while it solves this issue, I think there are deeper issues with the way the code is currently structured that need to be addressed.

Essentially, none of the operations (e.g. in your case, From(), but also e.g. Upsert()) should be modifying the global state of the shared client object. So any headers and params that are required should be passed down and set on the request object, rather than the client, to avoid correctness issues across re-use of the client.

Let me know if you'd like to fix that, else I'm happy to submit a PR.

intabulas commented 3 years ago

So I reverted that change (I forgot I still had the error handling PR open on same branch). But I agree and thats part also of why I reverted what I was working on. The thought was (like supabase-js) to push the table all the way down (Filter/Query/Transform Builders) and use it execute. I also wanted to get transform builders wired in so they could be used to. I was/am planning but I have a busy day job first half of this week so go for it