FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

🐛 Fix setting nullable property of Fields that don't accept `None` by van51 · Pull Request #79 · fastapi/sqlmodel · GitHub

  1. fastapi
  2. sqlmodel

Repository navigation

🐛 Fix setting nullable property of Fields that don't accept None - #79

Merged
tiangolo merged 3 commits into
fastapi:mainfrom
van51:bugfix/nullable_field
Aug 27, 2022
ConversationCommits (3)ChecksFiles changed
Merged

🐛 Fix setting nullable property of Fields that don't accept None#79
tiangolo merged 3 commits into
fastapi:mainfrom
van51:bugfix/nullable_field

Conversation

van51 commented Sep 6, 2021

Copy link
Copy Markdown
Contributor

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 commented Oct 7, 2021

Copy link
Copy Markdown

@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 commented Oct 7, 2021

Copy link
Copy Markdown

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

highflyingaction reviewed Nov 8, 2021
View reviewed changes
Comment thread sqlmodel/main.py Outdated

codecov Bot commented Aug 27, 2022 •
edited
Loading

Copy link
Copy Markdown

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 Bot commented Aug 27, 2022

Copy link
Copy Markdown
Contributor

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

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 Bot commented Aug 27, 2022

Copy link
Copy Markdown
Contributor

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

tiangolo commented Aug 27, 2022

Copy link
Copy Markdown
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 merged commit 9830ee0 into fastapi:main Aug 27, 2022
tiangolo mentioned this pull request Aug 27, 2022
Closed
8 tasks done
coderabbitai Bot mentioned this pull request Jul 13, 2026
Merged
coderabbitai Bot mentioned this pull request Aug 19, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tiangolo tiangolo left review comments
+1 more reviewer
highflyingaction highflyingaction left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

Back | FazBrowse Home | New Git URL