Re: foreign key locks

On Thu, Oct 18, 2012 at 04:58:20PM -0300, Alvaro Herrera wrote:
> Here is version 22 of this patch.  This version contains fixes to issues
> reported by Andres, as well as a rebase to latest master.

I scanned this for obvious signs of work left to do.  It contains numerous XXX
and FIXME comments.  Many highlight micro-optimization opportunities and the
like; those can stay.  Others preclude commit, either highlighting an unsolved
problem or wrongly highlighting a non-problem:

> +     /*
> +      * XXX we do not lock this tuple here; the theory is that it's sufficient
> +      * with the buffer lock we're about to grab.  Any other code must be able
> +      * to cope with tuple lock specifics changing while they don't hold buffer
> +      * lock anyway.
> +      */

>    * so they will be uninteresting by the time our next transaction starts.
>    * (XXX not clear that this is correct --- other members of the MultiXact
>    * could hang around longer than we did.  However, it's not clear what a
> !  * better policy for flushing old cache entries would be.)  FIXME actually
> !  * this is plain wrong now that multixact's may contain update Xids.

> !     nmembers = GetMultiXactIdMembers(multi, &members, true);
> !     /*
> !      * XXX we don't have the infomask to run the required consistency check
> !      * here; the required notational overhead seems excessive.
> !      */

>           /* We assume the cache entries are sorted */
> !         /* XXX we assume the unused bits in "status" are zeroed */
> !         if (memcmp(members, entry->members, nmembers * sizeof(MultiXactMember)) == 0)

> !  * XXX do we have any issues with needing to checkpoint here?
>    */
> ! static void
> ! TruncateMultiXact(void)
>   {

> +     /* FIXME what should we initialize this to? */
> +     newFrozenMulti = ReadNextMultiXactId();

> +      * FIXME -- the XMAX_IS_MULTI test is a bit wrong .. it's possible to
> +      * have tuples with that bit set that are dead.  However, if that's
> +      * changed, the RawXmax() call below should probably be researched as well.
>        */
>       if (tuple->t_infomask &
> !         (HEAP_XMAX_INVALID | HEAP_XMAX_LOCK_ONLY | HEAP_XMAX_IS_MULTI))
>           return false;