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;