Code review comment for lp:~adeuring/launchpad/bug-39674-lfa-editable

Revision history for this message
Abel Deuring (adeuring) wrote :

On 28.07.2010 02:01, Robert Collins wrote:
> A few notes - this is a great branch.
>
> The spike I'm doing on restricted librarian things will likely
> eventually benefit from the adapter.
>
> It would be great to do a quick check that we don't share LFA's
> anywhere; there isn't any need to share them because they themselves
> are the sharing mechanism for content in the librarian.

I already did a quick check and I think that LFAs referenced by bug
attachments are not referenced elsewhere. But:

1. There is an obvious difference betwen "I think" and "I am absolutely
sure"
2. Even if we were absolutely sure that we don't create such references
today, some code that creates a second reference to a bug attachment's
LFA this could in the future accidentally slip into our code base.

It would be nice if we could enforce this by a constraint on all columns
which reference LFAs, but according to
file:///usr/share/doc/postgresql-doc-8.4/html/sql-createtable.html :

> Currently, CHECK expressions cannot contain subqueries nor refer to variables other than columns of the current row.

We could perhaps add triggers for INSERTs and UPDATEs of all tables that
reference LFA which ensures that something like

  SELECT lfa_reference FROM table1 WHERE lfa_reference=this_reference
  UNION ALL
    SELECT lfa_reference FROM table2 WHERE lfa_reference=this_reference
  UNION ALL...

returns exactly one row.

« Back to merge proposal