🏷️ Adjust type annotation for `Field.sa_type` to support instantiated SQLAlchemy types

[diachkow]

[!NOTE] When I was writing description for this PR, I found another discussion started for just the same issue I was experiencing with mypy, so this changes are basically fixing the issue described here

Using sa_type and sa_column_kwargs instead of just sa_column can benefit when using inheritance for classes derived from SQLModel as it was suggested here.

If sa_column is not specified and sa_type is provided, it will be passed as a second, type argumnet to sqlalchemy.Column instance. If you would check sqlalchemy.Column construction definition, it looks as following:

    def __init__(
        self,
        __name_pos: Optional[
            Union[str, _TypeEngineArgument[_T], SchemaEventTarget]
        ] = None,
        __type_pos: Optional[
            Union[_TypeEngineArgument[_T], SchemaEventTarget]
        ] = None,
        *args: SchemaEventTarget,
        name: Optional[str] = None,
        type_: Optional[_TypeEngineArgument[_T]] = None,
        autoincrement: _AutoIncrementType = "auto",
        default: Optional[Any] = _NoArg.NO_ARG,
        insert_default: Optional[Any] = _NoArg.NO_ARG,
        doc: Optional[str] = None,
        key: Optional[str] = None,
        index: Optional[bool] = None,
        unique: Optional[bool] = None,
        info: Optional[_InfoType] = None,
        nullable: Optional[
            Union[bool, Literal[SchemaConst.NULL_UNSPECIFIED]]
        ] = SchemaConst.NULL_UNSPECIFIED,
        onupdate: Optional[Any] = None,
        primary_key: bool = False,
        server_default: Optional[_ServerDefaultArgument] = None,
        server_onupdate: Optional[_ServerOnUpdateArgument] = None,
        quote: Optional[bool] = None,
        system: bool = False,
        comment: Optional[str] = None,
        insert_sentinel: bool = False,
        _omit_from_statements: bool = False,
        _proxies: Optional[Any] = None,
        **dialect_kwargs: Any,
    ):

Note the __type_pos argument with Union[_TypeEngineArgument[_T], SchemaEventTarget] where

_TypeEngineArgument = Union[Type["TypeEngine[_T]"], "TypeEngine[_T]"]

So, from technical perspective you can pass not only the subclass of TypeEngine, e.g. SQLAlchemy's sqltype such as String, Integer, DateTime, JSON etc, but also an instance of this type.

I was trying for JSONB(none_as_null=True) and String(50) and it worked just fine, alembic migrations were generated correctly, only mypy was arguing for type mismatch with call-overload issue.

To fix mypy error, we can update type annotation for sqlmodel.main.Field.sa_type to support also an instantiated SQLAlchemy's sqltype to match those of sqlalchemy.Column

Related discussions:

• https://github.com/fastapi/sqlmodel/discussions/1228
• https://github.com/fastapi/sqlmodel/discussions/1669
• https://github.com/fastapi/sqlmodel/discussions/955

[diachkow]

Okay, the linting errors in CI seems to be unrelated to my PR. As for the label, I am unsure if its a feature or bug, can someone from maintainers assign a label, please 🙏

[svlandeg]

Hi @diachkow, thanks for the PR! We'll get back to you once the team has been able to review this in detail, I'll label it as feature for now.

the linting errors in CI seems to be unrelated to my PR

Yes - those are unrelated so no worries. They should be fixed once https://github.com/fastapi/sqlmodel/pull/1340 is merged.

[diachkow]

Thanks for approval! Who is responsible for merging this, what are the rules in this repo?

[YuriiMotov]

Thanks for approval! Who is responsible for merging this, what are the rules in this repo?

Only Sebastian can merge it. I already forwarded it to him. We should just wait