Skip to content

Conversation

@sambadi
Copy link

@sambadi sambadi commented Feb 12, 2024

Hello folks!
First of all i want to apologise for my bad english )
So i want to present two features realised for my project and what i want to share with community:
1)To be allow to change default Python to SqlAlchemy types mapping
2) To be allow to store Pydantic model data as Json at database (with ability to use SQLAlchemy Mutable wrap)

Possible is closes issue #503, but not sure

@github-actions
Copy link
Contributor

📝 Docs preview for commit 6b5f287 at: https://6b70ec7d.sqlmodel.pages.dev

@sambadi
Copy link
Author

sambadi commented Feb 12, 2024

Looks like in new version of pydantic (2.6.1) added some breaking changes. All completed tests at MR pipeline was started with previous stable version of pydantic - 2.5.3

This commit at Pydantic.main totally breaks the eq behaviour

pydantic/pydantic@62ed0aa#diff-8708af8d26c26644976866f974e0b810b0940ede43ff487e457057d5a1f07999

See discussion about _eq at #761

@github-actions
Copy link
Contributor

📝 Docs preview for commit fb58b86 at: https://db9f51b8.sqlmodel.pages.dev

@alejsdev alejsdev added the feature New feature or request label Jul 12, 2024
@KrilleGH
Copy link

That looks interesting. I'm using Pydantic's NaiveDatetime, and since it isn't a subclass of datetime.datetime I have to pass sa_type=saDateTime (from sqlalchemy import DateTime as saDateTime) on every field. Otherwise get_sqlalchemy_type() wouldn't know how to handle this type in the database.

However I think this case should be handled internally by SQLModel and without requiring extra code since SQLModel already depends on Pydantic and could just as well support these types.

@github-actions github-actions bot added the conflicts Automatically generated when a PR has a merge conflict label Sep 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2025

This pull request has a merge conflict that needs to be resolved.

Copy link
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

@sambadi, thank you for your interest and efforts!

So, the idea of this PR is that instead of having a bunch of IFs in get_sqlalchemy_type we can create a sa_types_map dict where we can store all mapping information. Then in get_sqlalchemy_type we would iterate through sa_types_map and choose the correct type if present.

This will allow users to re-map type for their apps in one place instead of providing sa_type for every field of that type.

I personally like the idea in general.
Let's wait for Sebastian to validate the idea

@KrilleGH
Copy link

@sambadi, thanks for implementing this! I considered doing it myself one day, but now I don't have to. ❤️
Could you please rebase the branch and resolve the conflicts so we can move on easier?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conflicts Automatically generated when a PR has a merge conflict feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants