Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Version history
- Temporarily restrict SQLAlchemy version to 2.0.41 (PR by @sheinbergon)
- Fixes ``add_import`` behavior when adding imports from sqlalchemy and overall better
alignment of import behavior(s) across generators
- Fixes ``nullable`` column behavior for non-null columns for both
``sqlmodels`` and ``declarative`` generators (PR by @sheinbergon)

**3.0.0**

Expand Down
8 changes: 6 additions & 2 deletions src/sqlacodegen/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,9 @@ def render_column(
args = []
kwargs: dict[str, Any] = {}
kwarg = []
is_sole_pk = column.primary_key and len(column.table.primary_key) == 1
is_part_of_composite_pk = (
column.primary_key and len(column.table.primary_key) > 1
)
dedicated_fks = [
c
for c in column.foreign_keys
Expand Down Expand Up @@ -460,8 +462,10 @@ def render_column(
kwargs["key"] = column.key
if is_primary:
kwargs["primary_key"] = True
if not column.nullable and not is_sole_pk and is_table:
if not column.nullable and not column.primary_key:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible for some columns of a multi-column primary key constraint to be NULL, though this is exceedingly rare in practice. But this is why I had the condition in place. Perhaps we could simply drop the is_table condition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm. I do understand this concern. However, this resulted in a somewhat weird behavior where some composite primary keys columns were getting the nullability flag. Maybe I misunderstood what I was seeing. I'll return this flag and check again

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we have a test covering the case of partially nullable PK, but it might be worth adding one if we don't have it already.

Copy link
Collaborator Author

@sheinbergon sheinbergon Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I realized what was bugging me. I've improved the conditioning and added a test to cover exactly the case you mentioned

kwargs["nullable"] = False
if column.nullable and is_part_of_composite_pk:
kwargs["nullable"] = True

if is_unique:
column.unique = True
Expand Down
6 changes: 3 additions & 3 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class Foo(Base):
__tablename__ = 'foo'

id: Mapped[int] = mapped_column(Integer, primary_key=True)
name: Mapped[str] = mapped_column(Text)
name: Mapped[str] = mapped_column(Text, nullable=False)
"""
)

Expand Down Expand Up @@ -115,7 +115,7 @@ class Foo(Base):
__tablename__ = 'foo'

id: Mapped[int] = mapped_column(Integer, primary_key=True)
name: Mapped[str] = mapped_column(Text)
name: Mapped[str] = mapped_column(Text, nullable=False)
"""
)

Expand All @@ -142,7 +142,7 @@ def test_cli_sqlmodels(db_path: Path, tmp_path: Path) -> None:

class Foo(SQLModel, table=True):
id: int = Field(sa_column=Column('id', Integer, primary_key=True))
name: str = Field(sa_column=Column('name', Text))
name: str = Field(sa_column=Column('name', Text, nullable=False))
"""
)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_generator_dataclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Simple(Base):
__tablename__ = 'simple'

id: Mapped[int] = mapped_column(Integer, primary_key=True)
age: Mapped[int] = mapped_column(Integer)
age: Mapped[int] = mapped_column(Integer, nullable=False)
name: Mapped[Optional[str]] = mapped_column(String(20), \
server_default=text('foo'))
""",
Expand Down
34 changes: 31 additions & 3 deletions tests/test_generator_declarative.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ class SimpleItems(Base):

id: Mapped[int] = mapped_column(Integer, primary_key=True)
top_container_id: Mapped[int] = \
mapped_column(ForeignKey('simple_containers.id'))
mapped_column(ForeignKey('simple_containers.id'), nullable=False)
parent_container_id: Mapped[Optional[int]] = \
mapped_column(ForeignKey('simple_containers.id'))

Expand Down Expand Up @@ -812,6 +812,34 @@ class SimpleItems(Base):
)


def test_composite_nullable_pk(generator: CodeGenerator) -> None:
Table(
"simple_items",
generator.metadata,
Column("id1", INTEGER, primary_key=True),
Column("id2", INTEGER, primary_key=True, nullable=True),
)
validate_code(
generator.generate(),
"""\
from typing import Optional

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

class Base(DeclarativeBase):
pass


class SimpleItems(Base):
__tablename__ = 'simple_items'

id1: Mapped[int] = mapped_column(Integer, primary_key=True)
id2: Mapped[Optional[int]] = mapped_column(Integer, primary_key=True, nullable=True)
""",
)


def test_joined_inheritance(generator: CodeGenerator) -> None:
Table(
"simple_sub_items",
Expand Down Expand Up @@ -1045,7 +1073,7 @@ class Group(Base):
)

groups_id: Mapped[int] = mapped_column(Integer, primary_key=True)
group_name: Mapped[str] = mapped_column(Text(50))
group_name: Mapped[str] = mapped_column(Text(50), nullable=False)

users: Mapped[list['User']] = relationship('User', back_populates='group')

Expand Down Expand Up @@ -1590,7 +1618,7 @@ class WithItems(Base):
__tablename__ = 'with_items'

id: Mapped[int] = mapped_column(Integer, primary_key=True)
int_items_not_optional: Mapped[list[int]] = mapped_column(ARRAY(INTEGER()))
int_items_not_optional: Mapped[list[int]] = mapped_column(ARRAY(INTEGER()), nullable=False)
str_matrix: Mapped[Optional[list[list[str]]]] = mapped_column(ARRAY(VARCHAR(), dimensions=2))
""",
)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_generator_sqlmodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_indexes(generator: CodeGenerator) -> None:
"item",
generator.metadata,
Column("id", INTEGER, primary_key=True),
Column("number", INTEGER),
Column("number", INTEGER, nullable=False),
Column("text", VARCHAR),
)
simple_items.indexes.add(Index("idx_number", simple_items.c.number))
Expand All @@ -58,8 +58,8 @@ class Item(SQLModel, table=True):
)

id: int = Field(sa_column=Column('id', Integer, primary_key=True))
number: Optional[int] = Field(default=None, sa_column=Column(\
'number', Integer))
number: int = Field(sa_column=Column(\
'number', Integer, nullable=False))
text: Optional[str] = Field(default=None, sa_column=Column(\
'text', String))
""",
Expand Down