Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve PostgreSQL's UUID support #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bochecha
Copy link

SQLAlchemy allows setting UUID columns to either a string representation
of a UUID (e.g '46260785-9b7e-4a59-824f-af994a510673') or to a Python
uuid.UUID object.

Fixes #94

SQLAlchemy allows setting UUID columns to either a string representation
of a UUID (e.g '46260785-9b7e-4a59-824f-af994a510673') or to a Python
uuid.UUID object.

Fixes dropbox#94
@rafales
Copy link
Contributor

rafales commented Jul 26, 2019

While this fixes your particular use case - it will break stuff for other people. Now all UUID fields on a model will be an union of str and UUID and client code will need to handle those cases.

I think the right solution here is to extend mypy plugin.
Basically what needs to be done is to hook into get_function_hook and return proper type (Instance with type argument set to str or UUID ) based on as_uuid argument.

@JukkaL
Copy link
Contributor

JukkaL commented Jul 26, 2019

I wonder if it would be possible to use an overloaded function with literal types to fix this? Maybe something like subprocess.Popen: https://github.com/python/typeshed/blob/master/stdlib/3/subprocess.pyi#L809

@rafales
Copy link
Contributor

rafales commented Jul 26, 2019 via email

@ilevkivskyi
Copy link
Contributor

I think we should try what @JukkaL proposes, using a union in this context may cause many false positives.

@bochecha
Copy link
Author

bochecha commented Aug 9, 2019

I wonder if it would be possible to use an overloaded function with literal types to fix this? Maybe something like subprocess.Popen: https://github.com/python/typeshed/blob/master/stdlib/3/subprocess.pyi#L809

It seems I took too long to reply and this line moved? Otherwise I don't see how line 809 of current master relates to this. 😕

I guess this is what you had in mind? https://github.com/python/typeshed/blob/3ad3ed82c75b4fb82f71f5500b50f18a98f12f49/stdlib/3/subprocess.pyi#L809

I just tried the following:

diff --git a/sqlalchemy-stubs/dialects/postgresql/base.pyi b/sqlalchemy-stubs/dialects/postgresql/base.pyi
index d910e5c..2c6917f 100644
--- a/sqlalchemy-stubs/dialects/postgresql/base.pyi
+++ b/sqlalchemy-stubs/dialects/postgresql/base.pyi
@@ -1,8 +1,9 @@
 from ... import schema
 from ...engine import default, reflection
 from ...sql import compiler, expression, sqltypes, type_api
-from typing import Any, Optional, Set, Type, Text, Pattern, Dict
+from typing import Any, Optional, Set, Type, Text, Pattern, Dict, overload
 from datetime import timedelta
+import uuid
 
 from sqlalchemy.types import INTEGER as INTEGER, BIGINT as BIGINT, SMALLINT as SMALLINT, VARCHAR as VARCHAR, \
     CHAR as CHAR, TEXT as TEXT, FLOAT as FLOAT, NUMERIC as NUMERIC, \
@@ -66,12 +67,20 @@ class BIT(sqltypes.TypeEngine[str]):
     def __init__(self, length: Optional[int] = ..., varying: bool = ...) -> None: ...
 PGBit = BIT
 
+@overload
 class UUID(sqltypes.TypeEngine[str]):
     __visit_name__: str = ...
     as_uuid: bool = ...
     def __init__(self, as_uuid: bool = ...) -> None: ...
     def bind_processor(self, dialect: Any): ...
     def result_processor(self, dialect: Any, coltype: Any): ...
+@overload
+class UUID(sqltypes.TypeEngine[uuid.UUID]):
+    __visit_name__: str = ...
+    as_uuid: bool = ...
+    def __init__(self, as_uuid: bool = ...) -> None: ...
+    def bind_processor(self, dialect: Any): ...
+    def result_processor(self, dialect: Any, coltype: Any): ...
 PGUuid = UUID
 
 class TSVECTOR(sqltypes.TypeEngine[str]):

However that doesn't work (same error message as #94). It seems @overload only works for functions and methods, not classes.

@ilevkivskyi
Copy link
Contributor

You need to overload the constructor, not the class itself.

@tdamsma
Copy link
Contributor

tdamsma commented Nov 12, 2020

@ilevkivskyi I made #185 to overload the constructor as per your suggestion. It seems to be pretty quiet here, it almost seems abandoned. Is it time for a community fork or is this just a temporary thing?

@ccrvlh
Copy link

ccrvlh commented Jun 24, 2021

Any news?
I have a column uuid = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid4)
When I pass an uuid while instantiating the model I get a warning saying that the uuid is expecting Optional[str].

@tdamsma
Copy link
Contributor

tdamsma commented Jun 24, 2021 via email

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mbridon
Copy link

mbridon commented Apr 17, 2022

Hi, I'm @bochecha in this ticket, but due to $circumstances I had to create a new account under my real name instead of my nickname.

I'm still happy to see people interested in following on this ticket and getting a way to find its way in, but if typing is now maintained by @zzzeek upstream I'm not sure we even need this ticket anymore ?

Sorry about not giving any sign of life this whole time.

($circumstances being my being hospitalized for 2 years and having to give up on all my floss activities during that time 😞)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using uuid.UUID with PotgreSQL's UUID
9 participants