postgrest-py icon indicating copy to clipboard operation
postgrest-py copied to clipboard

`_TableT` is not compatible with `TypedDict`

Open Glinte opened this issue 6 months ago • 3 comments
trafficstars

Bug report

  • [x] I confirm this is a bug with Supabase, not with my own application.
  • [x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

https://github.com/supabase/postgrest-py/blob/576a5b84e19c0a379ef10df24f3325c0519d406e/postgrest/_async/client.py#L15-L21

_TableT should be ~~MutableMapping[str, Any]~~ (Edit: Mapping[str, Any]) instead of dict[str, Any] to allow for users to specify a TypedDict as the output type. This seems to be a new-ish regression because my type checker wasn't complaining 4 months ago (edit: this is because there was no _TableT in postgrest 0.19.3 )

Issues from the two main type checkers explaining why dict is not compatible with TypedDict by design mypy: https://github.com/python/mypy/issues/4976 pyright: https://github.com/microsoft/pyright/issues/6658

To Reproduce

Image

async def f():
    db = AsyncClient(supabase_url, supabase_key, options)
    response: APIResponse[MessageRecord] = await db.table("messages").select("*").execute()

class MessageRecord(TypedDict):
    content: str | None

Expected behavior

Using a TypedDict should be accepted

System information

  • OS: Windows
  • postgrest-py: 1.0.1

Glinte avatar May 16 '25 04:05 Glinte

I've had a look at this and even with the change your propose it's still not accepting the new type defined on the returned response. What I've found to work is to not worry about the type at the .execute() level but rather do the type conversion with a Pydantic model by dumping the response to a model.

import os
import asyncio
from typing import Annotated

from dotenv import load_dotenv
from postgrest.exceptions import APIError
from postgrest._async.client import AsyncPostgrestClient
from pydantic import BaseModel, Field

load_dotenv()

url = os.environ.get("SUPABASE_URL", "")
key = os.environ.get("SUPABASE_KEY", "")

class Country(BaseModel):
    id: Annotated[int, Field(alias="id")]
    name: Annotated[str, Field(alias="name")]

def Countries(data: list):
    return [Country(**x) for x in data]

class CountryResponse(BaseModel):
    count: Annotated[int | None, Field(alias="count")]
    data: Annotated[list[Country], Field(alias="data")]

async def main():
    headers = {"Authorization": f"Bearer {key}"}
    async with AsyncPostgrestClient(base_url=f"{url}/rest/v1", headers=headers) as client:
        try:
            resp = (await client.table("countries").select("*").execute()).model_dump()
            response = CountryResponse(**resp)

            for row in response.data:
                print(f"RESPONSE: {row.id} {row.name}")
        except APIError as e:
            print(f"ERROR: {e}")

asyncio.run(main())

This might not be ideal for you but this is the current working solution I could come up with. I will investigate more to see how we can coerce the type a bit more to follow your original setup.

silentworks avatar May 16 '25 22:05 silentworks

Seems to be impossible to do inline type hint unless PEP 718 Subscriptable functions is accepted. I checked by reverted to the old version of postgrest that I was using, the reason why it worked before is because there was no _TableT so pyright just assumed Unknown i.e. Any in another name, and allows me to subscript with whatever type I want.

Reason why I believe coercing the type to my original setup is impossible is because

_TableT = Dict[str, Any]
AsyncPostgrestClient.from_(self, table: str) -> AsyncRequestBuilder[_TableT]: ...

has defined already defined the output type through the chain of classes, which passes _TableT into APIResponse[_TableT]

class APIResponse(BaseModel, Generic[_ReturnT]):
    data: List[_ReturnT]
    """The data returned by the query."""

and _ReturnT is invariant because data is list[_ReturnT] and the type parameter inside lists is invariant. What is being done right now means APIReponse.data must always be assumed to be Dict[str, Any]. Nothing that is done afterwards short of a cast can matter

Here are 3 ways that I came up that was close to what I wanted

  1. Split into two lines. I feel like this could work, but I don't understand why it isn't working.
table: AsyncRequestBuilder[MessageRecord] = db.table("messages")
response = await table.select("*").execute()
  1. Wrapping over AsyncClient/Client to add custom overloads to tables for each known table_name. This still requires _TableT = Mapping[str, Any]. One small issue is that it is possible to select only a subset of the columns, so the TypedDict has to be total=False
class DatabaseManager(AsyncClient):
    """Wrapper class for the supabase client."""
    @overload
    def table(self, table_name: Literal["messages"]) -> AsyncRequestBuilder[MessageRecord]: ...

    @overload
    def table(self, table_name: str) -> AsyncRequestBuilder[Mapping[str, Any]]: ...

    @override
    def table(self, table_name: str) -> AsyncRequestBuilder[Mapping[str, Any]]:
        """Override the table method to add the correct type hints via overloads."""
        return self.postgrest.from_(table_name)
  1. Hope PEP 718 Subscriptable functions comes out and be able to do, changing _TableT to a TypeVar with default
response = await db.table[MessageRecord]("messages").select("*").execute()

Glinte avatar May 19 '25 14:05 Glinte

Generating a new notification as I finished investigating.

I don't think my approach works with what you guys have without quite a large refactoring and complex typing that includes the default keyword for TypeVar that is only introduced in 3.13.

Since I already have a small fork of postgrest to fix supabase/supabase-py#1207 , I have decided that I will just add a commit that revert the typing and set _TableT = Any.

I would close the issue as not planned, but leaving it to you to decide whether to keep the issue and (eventually) support my usage

Glinte avatar Jun 02 '25 03:06 Glinte

TableT had been added a while back - introduced in supabase/postgrest-py#309 . I find it odd that you're only running into this issue now

what you guys have without quite a large refactoring and complex typing that includes the default keyword for TypeVar that is only introduced in 3.13.

@Glinte that was indeed the plan when I'd written that; see the discussion in supabase/postgrest-py#301 for some background

Split into two lines. I feel like this could work, but I don't understand why it isn't working.

This doesn't work because client.from_ explicitly declares that it returns a RequestBuilder[_TableT] (and as TableT = dict[str, Any]); so typehinting as RequestBuilder[MessageRecord] ends up being wrong

The simplest approach I see here is for us to add another method, one that can accept the eventual return type as an argument and then pass it along the query builder chain (you can see I had a similar function in the PR I'd linked here https://github.com/supabase/postgrest-py/issues/301#issuecomment-1721343554)

I was able to get something like that working: After adding:

   # _T defined as a plain TypeVar before this use
    def typed_from(self, table: str, ret_type: type[_T]) -> SyncRequestBuilder[_T]:
        return SyncRequestBuilder[ret_type](self.session, f"/{table}")

(method name to be bikeshedded!), the following works with pyright:

Image

This would be a deviation from the JS client API I guess, but I don't see how it'd be an issue considering it's just a new method being added.

Just thought I'd give my two cents

anand2312 avatar Jul 22 '25 14:07 anand2312

@anand2312 typed_from is fundamentally wrong (unsound), the earliest possible moment to determine the return type is at select because you can do join selects [1] or not select the whole table. Edit: Oh, I see the way you worked around it by defining the join relationships in the models. That would work yeah. For TypedDicts it would require the user to remember to use total=False though. If possible, it still seems optimal to use type hints and make the type information back-propagate. i.e. the way I originally did it.

response: APIResponse[MessageRecord] = await db.table("messages").select("*").execute()

[1] https://supabase.com/docs/reference/python/select

response = (
    supabase.table("users")
    .select("name, teams(name)")
    .execute()
)

Glinte avatar Jul 23 '25 02:07 Glinte

@Glinte right, we're working with the assumption that the type that the user passes in should be representative of the data returned by the entire query (so it should account for any joins and such). Any kind of inference that would reflect that the returned data only has some specific selected columns/dynamically include fields from joined tables etc (both of which would work in typescript, if I'm not wrong) is just not possible currently with Python's typing ecosystem. So yeah, we'd need something like typeddict's with total=False / pydantic models with all fields being optional etc

Type-hinting the final response like you've shown does not back-propagate that type information anywhere; at most it signals to your type checker that this is the type of this variable. But that type annotation is just incorrect, because the type variable T in APIResponse[T] is invariant, so even if TypedDict is type compatible with Mapping[str, Any] the type checker will complain that your typeddict is not the same type as Mapping[str, Any]; here's a simplified example of the situation: pyright playground (edit: re-reading your earlier comment, you already know that haha; I wouldn't hold my breath for PEP 718, I don't think it's gonna get accepted)

The benefit to actually passing the type down the query builder chain like in the example I'd given earlier is that pydantic will use that type to actually validate/coerce the response into that type, so you can be sure that the final object you're getting is of the right type (or pydantic will throw a validationerror)

anand2312 avatar Jul 24 '25 18:07 anand2312