-
-
Notifications
You must be signed in to change notification settings - Fork 752
✨ Allow to use Enums in custom DB schema #1146
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
base: main
Are you sure you want to change the base?
Conversation
45e843c
to
93d3756
Compare
810fa1c
to
b5132b5
Compare
Hey @svlandeg, I've rebased the PR and all checks are green now. I'd be ready for review. |
b5132b5
to
e37df31
Compare
LGTM but what would the advantage of this be instead of using |
@Seluj78, I'm not sure, where would you use My database is Postgres (I edited the description to mention this clearly) and it's possible to have different schemas there. This kind of schema is not to be confused with the structure of the database tables, also sometimes called schema. This schema is a way to group tables and other database objects. Without specification everything goes into the default I have a lot of stuff in the While SQLModel normally creates everything in the |
Oh my, I'm really sorry I completely misread and misunderstood your PR. After re-reading it, it totally makes sense. Sorry again! |
@KrilleGH, thanks for your interest and efforts! As I understand it, In SQLAlchemy we need to explicitly specify My main question is: will it be correct that in SQLModel this will be by-default? Can it be breaking change if somebody has multiple schemas and wants to have global enum type? For now you can go the same way as with SQLAlchemy and specify class Hero(SchemaSQLModel):
type: HeroType = Field(sa_type=Enum(HeroType, inherit_schema=True)) Results in:
Runnable code example in the details: import enum
from sqlalchemy.schema import CreateSchema
from sqlmodel import Enum, Field, SQLModel, create_engine
class SchemaSQLModel(SQLModel):
__table_args__ = {
'schema': 'api',
}
class HeroType(enum.StrEnum):
normal = enum.auto()
super = enum.auto()
class Hero(SchemaSQLModel, table=True):
id: int | None = Field(default=None, primary_key=True)
type: HeroType = Field(sa_type=Enum(HeroType, inherit_schema=True))
engine = create_engine("postgresql+psycopg2://user:mysecretpassword@localhost:5432/some_db", echo=True)
SQLModel.metadata.drop_all(engine)
with engine.connect() as connection:
connection.execute(CreateSchema("api", if_not_exists=True))
connection.commit()
SQLModel.metadata.create_all(engine) For now I would vote for not changing this and advise to explicitly specify |
@YuriiMotov, thank you for the suggestion. That looks like a good alternative I didn't find. I'll try it. I don't have enough information to comment on your question regarding the difference in behavoir of SQLAlchemy and SQLModel this would create. I currently have the patch from this PR automatically installed into my Docker container. I think I'd have to put an explicit |
I have a legacy Postgres DB which is receiving an API including new tables in a separate DB schema. The new code must not touch the existing objects in the
public
schema.So all models derive from
I have some fields which use Enums:
The DB is PostgreSQL and I'm using the native Enum type (
CREATE TYPE
). Also these types reside in the custom schema.When creating a new
Hero
the generated SQL has explicit casts to the types of the columns. However, for thetype
column only the plain name of the type is generated. Of course that doesn't work, as the Enum type isn't in thepublic
schema.The solution is to pass
inherit_schema=True
when instantiating SQLAlchemy'sEnum
class. I achived this by making a subclass and using that inget_sqlalchemy_type()
instead of the plain one.I'm aware this PR may not be in it's final form. Please advise what needs to be done to get it merged.