tiangolo / sqlmodel

SQL databases in Python, designed for simplicity, compatibility, and robustness.
https://sqlmodel.tiangolo.com/
MIT License
13.7k stars 616 forks source link

[BUG] Variables with annotation of 'typing.Literal' causes a panic #57

Open faresbakhit opened 2 years ago

faresbakhit commented 2 years ago

First Check

Commit to Help

Example Code

from sqlmodel import SQLModel
from typing import Literal

class Cool(SQLModel, table=True):
    name: Literal["fastapi", "sqlmodel"]

# This code outputs the following to stderr

#   File "main.py", line 5, in <module>
#     class Cool(SQLModel, table=True):
#   File "sqlmodel/main.py", line 292, in __new__
#     col = get_column_from_field(v)
#   File "sqlmodel/main.py", line 415, in get_column_from_field
#     sa_type = get_sqlachemy_type(field)
#   File "sqlmodel/main.py", line 373, in get_sqlachemy_type
#     if issubclass(field.type_, str):
# TypeError: issubclass() arg 1 must be a class

Description

This happens because typing.Literal[...] is a function and since SQLModel uses issubclass to get the variable's SQLAlchemy type; issubclass just throws a TypeError since it expects a class.

Operating System

Linux

Operating System Details

No response

SQLModel Version

0.0.4

Python Version

Python 3.9.6

Additional Context

Funny enough Pydantic had quite the same issue samuelcolvin/pydantic#1026 so it can be as easy as importing pydantic.typing.is_literal_type and running an if statement at the top of get_sqlachemy_type

I think the real issue is that typing.Literal is an 'Union' type it receive one or more values but since it's often used for one type (e.g. Literal["r", "w", ...]) we can return the type of all the passed values if they are all the same else None. The values can be retrieved with typing.get_args as a tuple.

shifqu commented 2 years ago

I also encountered this issue and ended up implementing the check for a Literal at the beginning of get_sqlachemy_type.

field_type = field.type_
if is_literal_type(field_type):
    # Try to get the single type of the literal, error otherwise
    types = list({type(x) for x in get_args(field_type)})
    if len(types) != 1:
        raise RuntimeError("Cannot map a literal with multiple types to a sql type.")
    field_type = types[0]

To make this work you would still need to import get_args and is_literal_type, also refactor each field.type_ call to field_type

The problem with this solution is that it is specifically for a Literal. So, not sure if this will be immediately accepted to be implemented. Happy to make this into a PR if needed.

I eventually stopped using Literal, and chose for an Enum, which works out of the box :)

In your case:

from enum import Enum

class NameEnum(str, Enum):
    FASTAPI = "fastapi"
    SQLMODEL = "sqlmodel"

class Cool(SQLModel, table=True):
    name: NameEnum
data-djinn commented 2 years ago

@shifqu this solved my problem, thanks!

Cielquan commented 1 year ago

I have the same issue and went with the enum solution. But the Literal approach would be still worth adding I guess, for the future to open up the possibilities.

Cielquan commented 1 year ago

I encountered an issue with the enum approach (see #406) where the validation for the field with the enum values silently passes for invalid values.

sasilver75 commented 1 year ago

I have this issue when trying to use a Discriminated Union on a SQLModel a la Pydantic

sietse commented 1 year ago

EDIT: See https://github.com/pydantic/pydantic/issues/5821#issuecomment-1559196859 for the main issue and a list of workarounds.

I'm deleting the rest of my comment: all its information is present in the linked thread.

PaleNeutron commented 4 months ago

In your case:

from enum import Enum

class NameEnum(str, Enum):
    FASTAPI = "fastapi"
    SQLMODEL = "sqlmodel"

class Cool(SQLModel, table=True):
    name: NameEnum

I have to emphasis this code actually write FASTAPI uppercase, the enum key into database, not "fastapi" lower case, the value.