sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.73k stars 239 forks source link

sqlachemy 2.0 nullability not reflecting properly with the alembic. #1508

Closed sant527 closed 3 weeks ago

sant527 commented 1 month ago

Describe the bug I see some descripency with nullability w.r.t sqlachemy sqlalchemy = "^2.0.30"

Expected behavior

As per the docs: https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#mapped-column-derives-the-datatype-and-nullability-from-the-mapped-annotation

from typing import Optional

from sqlalchemy.orm import DeclarativeBase
from sqlalchemy.orm import Mapped
from sqlalchemy.orm import mapped_column

class Base(DeclarativeBase):
    pass

class SomeClass(Base):
    __tablename__ = "some_table"

    # primary_key=True, therefore will be NOT NULL
    id: Mapped[int] = mapped_column(primary_key=True)

    # not Optional[], therefore will be NOT NULL
    data: Mapped[str]

    # Optional[], therefore will be NULL
    additional_info: Mapped[Optional[str]]

data: Mapped[str] should create a column with not null as per the above docs. but i see alembic doing

op.add_column('some_table', sa.Column('data', sa.String(), nullable=True))

I am expecting the column will be not null type or nullable = False

To Reproduce Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved. See also Reporting Bugs on the website.

# Insert code here

Error

# Copy error here. Please include the full stack trace.

Versions.

Additional context

Have a nice day!

zzzeek commented 3 weeks ago

hi this cannot be reproduced

using to start:

first version:

    class SomeClass(Base):
        __tablename__ = "some_table"

        # primary_key=True, therefore will be NOT NULL
        id: Mapped[int] = mapped_column(primary_key=True)

        # Optional[], therefore will be NULL
        additional_info: Mapped[Optional[str]]

migration

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('some_table',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('additional_info', sa.String(), nullable=True),
    sa.PrimaryKeyConstraint('id')
    )
    # ### end Alembic commands ###

second version, add data without Optional

    class SomeClass(Base):
        __tablename__ = "some_table"

        # primary_key=True, therefore will be NOT NULL
        id: Mapped[int] = mapped_column(primary_key=True)

        # not Optional[], therefore will be NOT NULL
        data: Mapped[str]

        # Optional[], therefore will be NULL
        additional_info: Mapped[Optional[str]]

migration, column is added nullable=False

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.add_column('some_table', sa.Column('data', sa.String(), nullable=False))
    # ### end Alembic commands ###

please use discussions for usage questions thanks!