sqlmodel icon indicating copy to clipboard operation
sqlmodel copied to clipboard

Add foreign_key name to wrapper

Open mkarbo opened this issue 3 years ago • 6 comments

Due to no name on foreign key objects, Sqlalchemy will complain when trying to drop tables (CircularDependencyError). This PR adds name attribute to foreign key objects created by SQLModel.

The error happens inside sqlalchemy.sql.ddl module at line 968, e.g., when using Postgres trying to drop tables.

mkarbo avatar Feb 24 '22 10:02 mkarbo

Is there any appetite to get this into SQLModel, I am also having this problem and using sa_column isn't really working for me either, nor can I find any other sensible way to name my foreign keys using SQLModel

@nomagick: did you find a workaround I can use in the mean-time?

matsavage avatar May 11 '22 15:05 matsavage

Hi mat, I don\t know what workaround you are refering to - we ended up using a branch like this, and it solved our issues related to the problem I specified in the PR

mkarbo avatar May 11 '22 21:05 mkarbo

@mkarbo ah sorry, I was just meaning did you find another way to achieve this same result in the un-patched version of SQLModel

matsavage avatar May 12 '22 08:05 matsavage

No, unfortunately not - if you do or you see an improvement to my fork/PR please do let me know :)

mkarbo avatar May 12 '22 17:05 mkarbo

I was able to do this in the unpatched with the following:

table_two_id: Optional[int] = Field(
    sa_column=Column(
        Integer,
        ForeignKey(
            "table_two.id", name="fk-table_one-table_two-id"
        ),
        default=None,
        nullable=False,
    )

However this does not work with the BaseModel relationships pattern https://sqlmodel.tiangolo.com/tutorial/fastapi/relationships/ as the SQLModel Relationship is not recognized by SQLAlchemy

I have also seen that SQLModel will name an index when you add one to a column, but why not an FK 👀

matsavage avatar May 13 '22 08:05 matsavage

@matsavage the fact that indices but not foreign keys are named is what led to this PR, trying to mimick the logic from indices - I agree it would be great to have it part of a release

mkarbo avatar Jun 04 '22 17:06 mkarbo