warn for overlapping FKs with writing

[Nick Retallack]

I see there was already an issue about overlapping composite keys in the past, but maybe mine's different, as this issue still exists in 0.9.8. The issue occurs when you try to un-relate some records with overlapping keys.

Here's my example:

from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import Column, ForeignKey, Integer, String, ForeignKeyConstraint
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker, relationship

Base = declarative_base()

# Lets simulate a multi-tenant database system that doesn't use multiple schemas
class Tenant(Base):
    __tablename__ = 'tenant'
    id = Column(Integer, primary_key=True)
    name = Column(String)

# Something to be related to
class Friend(Base):
    __tablename__ = 'friend'
    id = Column(Integer, primary_key=True)
    tenant_id = Column(ForeignKey('tenant.id'), primary_key=True)
    name = Column(String)
    tenant = relationship(Tenant)

# Our model
class Person(Base):
    __tablename__ = 'person'
    id = Column(Integer, primary_key=True)
    tenant_id = Column(ForeignKey('tenant.id'), primary_key=True)
    friend_id = Column(Integer)
    name = Column(String)

    __table_args__ = (
        # friend relationship
        ForeignKeyConstraint(
            (friend_id, tenant_id),
            ('friend.id','friend.tenant_id'),
        ),
    )

    tenant = relationship(Tenant)
    friend = relationship(Friend)

engine = create_engine('sqlite:///:memory:', echo=True)
Session = sessionmaker(bind=engine)

def run():
    # Basic Setup
    Base.metadata.create_all(engine)
    session = Session()

    # Make a person with a friend
    tenant = Tenant(name="testing", id=1)
    person = Person(name="Bob", tenant=tenant, id=1)
    friend = Friend(name="George", tenant=tenant, id=1)
    person.friend = friend
    session.add_all([tenant, person, friend])
    session.commit()

    # Take away the friend
    person = session.query(Person).first()
    person.friend = None
    session.commit()
    # it tries to take away the tenant_id too, thus breaking the primary key

if __name__ == '__main__':
    run()

When I run it, I get this:

AssertionError: Dependency rule tried to blank-out primary key column 'person.tenant_id' on instance '<Person at 0x105c05310>'

It shouldn't be blanking out the tenant_id, since that id still references the tenant. The relationship needs to know that, despite joining on a composite key here, only one of those columns is specific to this relationship. The other is shared with many other relationships.

[Mike Bayer]

This is an extremely odd and advanced case but so far I think there is no bug here other than potentially lack of a warning.

You'd like a relationship where you can define specific foreign key elements that are written elsewhere (in this case Person.tenant_id is written by Person->Tenant only), however the relationship overall should not be viewonly (e.g. you still want to write into Person.friend_id). If you do in fact have more than one Friend row with the same "id" and different "tenant_id" (which would be interesting to confirm), then you do in fact need Person.friend to query on both Person.friend_id and Person.tenant_id in order to ensure the correct row.

To call this a bug means SQLAlchemy would know how to guess that, even though Person.tenant_id is used as a foreign key in two different relationships, only one of those relationships is the "real" one for which SQLAlchemy should be poking in an incoming (or being erased) "tenant_id" value.

For example. What if you had a Person, and you assign to it Tenant(id=5) and Person(id=2, tenant_id=7) ? Which one wins? (answer: whichever one you did last.) SQLAlchemy at best would at configruation time raise an error that this setup is ambiguous (and that is something that maybe SQLAlchemy could do here, to express that hey, you are putting this tenant_id column into a conflicting position).

Right now, forget about the removal of the friend. Here's another easy one:

tenant = Tenant(name="testing", id=1)
tenant2 = Tenant(name="testing", id=2)
person = Person(name="Bob", tenant=tenant, id=1)

friend = Friend(name="George", tenant=tenant2, id=1)
session.add_all([tenant, person, friend])

session.commit()  # INSERT: person tenant_id=1

person.friend = friend
session.commit()  # UPDATE:  person SET tenant_id=2, friend_id=1
assert person.tenant is tenant  # <-- nope

The model here is just leaving out a really critical detail, which is, which relationship gets to write into tenant_id?

To tell relationship() this information, you set up a primaryjoin along with which columns are foreign_keys at the same time:

from sqlalchemy.orm import foreign

class Person(Base):
    friend = relationship(
        Friend,
        primaryjoin=and_(
            foreign(friend_id) == Friend.id,
            tenant_id == Friend.tenant_id
        )
    )

now if we do the above test of Friend(id=1, tenant_id=2) / Tenant(id=1), with foreign keys enabled (I used Postgresql), we get the expected constraint violation when we try to associate the incongruous friend with Person:

UPDATE person SET friend_id=%(friend_id)s WHERE person.id = %(person_id)s AND person.tenant_id = %(person_tenant_id)s
{'person_id': 1, 'friend_id': 1, 'person_tenant_id': 1}

sqlalchemy.exc.IntegrityError: (psycopg2.IntegrityError) insert or update on table "person" violates foreign key constraint "person_friend_id_fkey"
DETAIL:  Key (friend_id, tenant_id)=(1, 1) is not present in table "friend".

and the original case works fine too:

person.friend = None
session.commit()
assert person.tenant is tenant
assert person.friend is None

The best SQLA could do here is warn that Person.tenant_id is being used in two contradictory writing contexts, though I think it becomes apparent that this is the case anyway (just less clearly).

[Mike Bayer]

i have a warning in the works for this.

[Mike Bayer]

• A warning is emitted in the case of multiple relationships that ultimately will populate a foreign key column in conflict with another, where the relationships are attempting to copy values from different source columns. This occurs in the case where composite foreign keys with overlapping columns are mapped to relationships that each refer to a different referenced column. A new documentation section illustrates the example as well as how to overcome the issue by specifying "foreign" columns specifically on a per-relationship basis. fixes #3230

→ <<cset 55cad302cee5>>

[Mike Bayer]

so this is a really elaborate check...done for every relationship unconditionally, and will virtually never come up for nearly anyone except the very few setups that do this kind of thing. then it just emits a warning ! :). but it's tough to catch this otherwise and it's a pretty nice message, here's yours:

/Users/classic/dev/sqlalchemy/lib/sqlalchemy/orm/relationships.py:2568: SAWarning: relationship 'Person.friend' will copy column friend.tenant_id to column person.tenant_id, which conflicts with relationship(s): 'Person.tenant' (copies tenant.id to person.tenant_id). Consider applying viewonly=True to read-only relationships, or provide a primaryjoin condition marking writable columns with the foreign() annotation.

pretty amazing warning!

[Nick Retallack]

Oh, ok, I didn't realize I could fix it by marking the foreign keys in the relationship like that. In fact, this is enough to get my example working:

friend = relationship(Friend, foreign_keys=[friend_id])

Thank you! Btw, where can I view the new documentation you mentioned?