Skip to content

Conversation

@van51
Copy link
Contributor

@van51 van51 commented Sep 6, 2021

Attempt to fix #76 where fields with default values would be considered nullable by sqlmodel, because they would be treated as not_required by Pydantic.

Previously

The following example code (copy-pasted from the issue)

from typing import Optional
from sqlmodel import SQLModel, Field, create_engine


class Test(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    a: str
    b: Optional[str] = None
    c: str = Field(default="Hey")
    d: str = "hey"
    
sqlite_url = ""  # define a path here
engine = create_engine(sqlite_url, echo=True)
SQLModel.metadata.create_all(engine)

used to produce the following create table SQL command:

CREATE TABLE test (
	id INTEGER, 
	a VARCHAR NOT NULL, 
	b VARCHAR, 
	c VARCHAR, 
	d VARCHAR, 
	PRIMARY KEY (id)
)

Now

This PR produces:

CREATE TABLE test (
	id INTEGER NOT NULL, 
	a VARCHAR NOT NULL, 
	b VARCHAR, 
	c VARCHAR NOT NULL, 
	d VARCHAR NOT NULL, 
	PRIMARY KEY (id)
)

Please let me know what you think.

@Brobin
Copy link

Brobin commented Oct 7, 2021

@tiangolo can we get this pulled in? nullable=False is a sensible default, especially for primary keys. I believe this will also address #119

@r614
Copy link

r614 commented Oct 7, 2021

^ I agree, I think this the query generated by this PR is more correct.

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #79 (33daef3) into main (2407ecd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #79   +/-   ##
=======================================
  Coverage   97.58%   97.59%           
=======================================
  Files         182      182           
  Lines        6054     6060    +6     
=======================================
+ Hits         5908     5914    +6     
  Misses        146      146           
Impacted Files Coverage Δ
sqlmodel/main.py 83.91% <100.00%> (+0.28%) ⬆️
...orial/test_create_db_and_table/test_tutorial001.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 2877c12 at: https://630a97760a0b955025267d1a--sqlmodel.netlify.app

@tiangolo tiangolo changed the title 🎨 Fix nullable property of Fields 🐛 Fix setting nullable property of Fields that don't accept None Aug 27, 2022
@github-actions
Copy link
Contributor

📝 Docs preview for commit 33daef3 at: https://630a97d81b8fa54cf7654d6b--sqlmodel.netlify.app

@tiangolo
Copy link
Member

Awesome, thank you @van51! Great work! 🚀 🍰

And thanks @highflyingaction for the review and for spotting something to update! 🤓


Thanks everyone for the discussion. This will be available in the next version released in the next hours, SQLModel 0.0.7 🎉

@tiangolo tiangolo merged commit 9830ee0 into fastapi:main Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-optional property with default value is not translated to a non-nullable field in sqlalchemy

5 participants