Open Stack

Mike Bayer <mbayer at redhat.com> wrote:

> 
>> I'm not entirely sure what you've said above actually prevents coders
>> from relying on the constraints. Being careful about deleting all of the
>> child rows before a parent is good practice. I have seen code like this
>> in the past though:
>> 
>> try:
>> parent.delete()
>> except ForeignKeyFailure:
>> parent.children.delete()
>> parent.delete()
>> 
>> This means if you don't have the FK's, you may never delete the
>> children. Is this a bug? YES. Is it super obvious that it is the wrong
>> thing to do? No.
> 
> So the point you’re making here is that, if foreign key constraints are
> removed, poorly written code might silently fail. I’m glad we agree this is
> an issue!  It’s the only point I’m making.

I apologize for my snark here. The above code is wrong, and I think it is
obviously wrong. People working on this code should be familiar with
SQLAlchemy basics (at least having read the ORM tutorial), and that includes
the very easy to use features of relationship management.

Even if we are dealing with a version of the above that does not use
SQLAlchemy, it should be apparent that a DELETE should be emitted for the
child rows whether or not they’ve been tested as existing, if we are
deleting on the criteria of “parent_id”. Code like the above should ideally
never get through review, and if code like that exists right now, it should
be fixed.

What foreign key guarantees get us for the above would be for the much
more common case that someone emits a DELETE for the parent row
without being at all aware that there are dependent rows present.  That
silent failure leaves those child rows as orphans which will
lead to application failures when accessed, assuming the application 
also attempts to access the referenced parent.