maurerle / eralchemy2

Entity Relation Diagrams generation tool - DEPRECATED
https://github.com/eralchemy/eralchemy
Apache License 2.0
69 stars 16 forks source link

Table with compound primary key has wrong cardinality #21

Closed kasium closed 4 months ago

kasium commented 1 year ago

Example (SQLAlchemy 2.0)

from sqlalchemy import Column, ForeignKey, String
from sqlalchemy.orm import DeclarativeBase
from eralchemy2 import render_er

class Base(DeclarativeBase): pass

class Parent(Base):
    __tablename__ = "parent"
    id = Column(String(), primary_key=True)

class Child(Base):
    __tablename__ = "child"
    id = Column(String(), primary_key=True)
    parent = Column(String(), ForeignKey(Parent.id), primary_key=True, nullable=False))

render_er(Base, "out.svg")

The resulting graph tells a 1-1 relationship, but it's 1-0..n. As soon as primary_key=True of the column parent is changed to False, the cardinality is correct again.

https://github.com/maurerle/eralchemy2/blob/d289869c255b3566b2295a809b91f32053157513/eralchemy2/sqla.py#L23-L28 I guess that here some handling needs to be added checking of other primary key columns

maurerle commented 1 year ago

I tried something in here: https://github.com/maurerle/eralchemy2/commit/7211b447e88799c5b7800e8055c455db28fdb08a

but this does not really solve the issue if you have compound keys. For example it does not cover:

class Parent(Base):
    __tablename__ = "parent"
    id = Column(String(), primary_key=True)
    second_id = Column(String(), primary_key=True)

out where the other side should be '*' too or

class Parent(Base):
    __tablename__ = "parent"
    id = Column(String(), primary_key=True)
    secondid = Column(String(), primary_key=True)

class Child(Base):
    __tablename__ = "child"
    id = Column(String(), ForeignKey(Parent.secondid), primary_key=True)
    parent = Column(String(), ForeignKey(Parent.id), primary_key=True)

out

where we again should have only one 1-1 relation between the tables. I am open for a more comprehensive approach to tackle compound keys here.

kasium commented 1 year ago

Tuff one. What about adding handling specific to the corners cases, so e.g. all PKs are FKs to one table or the one initially reported in this issue. I have currently no good algorithm in mind which takles everything

maurerle commented 4 months ago

Yes @kasium - I have fixed this through some logic in #34 . It should now work as expected in all cases :) Thanks for the discussion! :)

You can get back to the issue if there is still something off, I will just close it for now.

kasium commented 4 months ago

Thanks a lot! Do you plan any release in the next time?