Skip to content

Easy to make mistakes when defining nullable (Field() vs Field(Column()) #464

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

Closed
8 tasks done
martin-greentrax opened this issue Oct 10, 2022 · 5 comments
Closed
8 tasks done
Labels
answered feature New feature or request question Further information is requested

Comments

@martin-greentrax
Copy link

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the SQLModel documentation, with the integrated search.
  • I already searched in Google "How to X in SQLModel" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to SQLModel but to Pydantic.
  • I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

Here a sample model:


class FooDB(SQLModel, table=True):
    id: int = Field(primary_key=True, nullable=False)
    datetime_1: datetime = Field(sa_column=Column(DateTime), nullable=False)
    datetime_2: datetime = Field(sa_column=Column(DateTime, nullable=False))

    string_1: str = Field(sa_column=Column(String(50)))
    string_2: str = Field(max_length=50)

Alembic creates the table as following:


    op.create_table(
        "foodb",
        sa.Column("datetime_1", sa.DateTime(), nullable=True),
        sa.Column("datetime_2", sa.DateTime(), nullable=False),
        sa.Column("string_1", sa.String(length=50), nullable=True),
        sa.Column("string_2", sqlmodel.sql.sqltypes.AutoString(length=50), nullable=False),
        sa.PrimaryKeyConstraint("id"),
    )



### Description

I had some surprises when looking closer at what data my tables actually accept vs. what I saw in my models.py. I find that it is very easy that one thinks one is doing the right thing (while looking at the pydantic/sqlmodel docs), but the definition does actually never make it into the database.

In my sample model I would have expected that the nullable-Attribute would lead to the same in each pair.
  
Related to https://github.com/tiangolo/sqlmodel/issues/420

### Operating System

Linux

### Operating System Details

_No response_

### SQLModel Version

0.0.8

### Python Version

Python 3.10.7

### Additional Context

_No response_
@martin-greentrax martin-greentrax added the question Further information is requested label Oct 10, 2022
@yixiongngvsys
Copy link

I overlooked your open issue, currently I am facing the same issue (#466) seems like if the nullable is set outside Column(), this field will be ignored

@tiangolo
Copy link
Member

If you use the SQLAlchemy column that overrides everything. It's made as an escape hatch so that you can take the wheel and SQLModel won't interfere.

@martin-greentrax
Copy link
Author

@tiangolo why is nullable=False in datetime_1 silently ignored though? To me that feels error prone :-|

@tiangolo
Copy link
Member

Thank you! It was indeed error-prone, I added #681 to solve it.

Now it's not possible to combine sa_column with other arguments that would be invalid/not used. It will raise an error. And it also includes the typing tricks so that the editor will complain right there while editing if a developer uses sa_column with one of those incompatible arguments.

It will be available in the next version, 0.0.11. 🎉

@tiangolo tiangolo added feature New feature or request answered labels Oct 29, 2023
Copy link

github-actions bot commented Nov 9, 2023

Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

@github-actions github-actions bot closed this as completed Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered feature New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants