fix: use SelectRequestBuilder for rpc call

[a5r0n]

fixes #200

[a5r0n]

@J0 @anand2312 can you take a look?

[a5r0n]

is there any reason to use async for this function?
await (await client.rpc(...)).execute()

Suggested change

	async def rpc(self, func: str, params: dict) -> AsyncSelectRequestBuilder:
	def rpc(self, func: str, params: dict) -> AsyncSelectRequestBuilder:

[codecov]

Codecov Report

Patch coverage: 66.66% and no project coverage change.

Comparison is base (ed7719c) 91.63% compared to head (8fc254a) 91.63%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #254   +/-   ##
=======================================
  Coverage   91.63%   91.63%           
=======================================
  Files          24       24           
  Lines        1183     1183           
=======================================
  Hits         1084     1084           
  Misses         99       99           

Impacted Files	Coverage Δ
postgrest/_async/client.py	90.00% <66.66%> (ø)
postgrest/_sync/client.py	90.00% <66.66%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[chrisgoddard]

Hi there @a5r0n! I'm also trying to use RPC calls with the postgres-py library -- I tried testing with your fork and called with the query.single().execute() syntax - but now I'm getting a different validation error:

data
  value is not a valid dict (type=type_error.dict)

When I look at the raw response that's being returned by PostgREST, it's just a raw value. For context, my function is an upsert_record function which takes a set of parameters to find or create a record and returns the id of the matched or created row.

In order for functions like this to be compatible with this implementation of postgrest-py, every function would have to return a custom type - which would mean that any other queries or functions within my database would have to accommodate that structure (for example, I have another similar upsert_* function that calls the first upsert_record function as part of it's operation - it would have to be changed to record_id := (select id from upsert_function(...)).

That's definitely doable (easier than always returning a table) - but it's still a bit of overhead just to accommodate the client.

Would an alternative approach be, perhaps, to provide in addition to the single method on an RPC query, a raw (or something similar) that lets the client return just the raw response from the RPC call without Pydantic validation so it can be used however we wish? Or, perhaps the single method could have a raw boolean parameter that would instruct it to just return the raw response rather than calling SingleAPIResponse.from_http_request_response

Thoughts?

[anand2312]

Sorry for not getting to this earlier
I'm leaning towards not having a pydantic model as the response of .rpc() here, and just return the JSON - feels like the most intuitive solution to me that covers every case. What do you guys think?
Okay so rpc returns a FilterRequestBuilder to allow filtering on the result of the rpc call, and that in turn ends up calling a unified "execute" method we have that returns the pydantic model in all cases - so just returning the json while also supporting filters will be a little more complicated than I'd initially thought.

@a5r0n sorry but I don't think using the SelectRequestBuilder fixes the issue here (correct me if I'm wrong)

[a5r0n]

from response perspective, there is 3 result types.

• when the function returns setof records, (e.g. CREATE FUNCTION find_users(...) RETURNS SETOF "User")
  for that, the current implementation works well.
• my use case - when the function returns object (e.g. CREATE FUNCTION get_user_id(..., OUT id integer))
• the last one, is when the function returns simple value, like what @chrisgoddard is looking for, (CREATE FUNCTION get_user_id(...) RETURNS integer)

[anand2312]

And I think functions can return None as well with RETURNS void iirc @a5r0n

[chrisgoddard]

My 2c for right now would be to at least provide the mechanism for returning the raw response (not the raw HTTP response - we can still call .json()) and then let users define how they want to use the response - I feel like there could be too many different edge cases of how people are using stored procedures within Postgres.

Right now I'm doing this as a workaround:

def rpc_call(
    supabase: SupabaseClient,
    procedure: str,
    params: dict = {},
    type_: Optional[type[BaseModel]] = None
):
    query = supabase.postgrest.rpc(
        procedure,
        orjson.loads(orjson.dumps(params))  # ensures json properly serialized for requests
    )
    data = query.session.request(
        query.http_method,
        query.path,
        json=query.json,
        params=query.params,
        headers=query.headers,
    ).json()

    if data:
        if type_ and isinstance(data, dict):
            return type_(**data)
        elif type_ and isinstance(data, list):
            return [type_(**o) for o in data]
        return data

Basic type responses (like int, string, etc) are still correctly parsed by .json - including correctly casting ints/floats (assuming you want that)

Ultimately I could imagine a more sophisticated solution where you could define a remote procedure explicitly in your client (could use Pydantic types) where the inputs to the function could also be validated as well as the response types - but for now I think just making it so people can use stored procedures however they have them configured would be the simplest.

[a5r0n]

@chrisgoddard the official supabase client is actually using a different library to invoke functions, did you try that? (https://github.com/supabase-community/functions-py)

[chrisgoddard]

@chrisgoddard the official supabase client is actually using a different library to invoke functions, did you try that? (https://github.com/supabase-community/functions-py)

My understand is that is for edge functions - I'm talking about Postgres stored procedures.

[a5r0n]

Sure, my mistake.

[a5r0n]

@anand2312
can we get this PR merged?
at least for my use case, this solution works fine.

[anand2312]

I'm reluctant to merge because it doesn't really fix the issue at hand 😦 If I'm not able to come up with something better in the next day or two I guess I'll merge this

[ShantanuNair]

@anand2312 @a5r0n Is there any update on this? Appreciate you guys working on this. I just built a bunch of stuff around the assumption that I Can use RPC calls and only now found out that this is broken on supabase-py/postgrest-py

[anand2312]

#309 was merged instead of this as it was a more complete fix - but thanks for the contribution regardless! @a5r0n