Date: Thu, 6 Feb 2020 20:01:29 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: FK violation in partitioned table after truncating a referenced
 partition
Message-ID: <20200206230129.GA9760@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200206223015.GA32754@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Feb-06, Alvaro Herrera wrote:

> On 2020-Feb-06, Alvaro Herrera wrote:
> 
> > I agree that patching heap_truncate_find_FKs is a reasonable way to fix.
> > I propose a slightly different formulation: instead of the loop that you
> > have, we can just use the second loop, and add more parent constraints
> > to the list if any constraint we scan in turn has a parent constraint.
> > So we don't repeat the whole thing, but only that second loop.
> 
> Hmm, this doesn't actually work; I modified your test case and I see
> that my code fails to do the right thing.

Yeah, AFAICS both algorithms posted so far (yours and mine) are wrong.
Maybe there's another way to fix it, but I think we're going to need the
find_all_inheritors call you didn't want; here's a rough sketch of what
I'm thinking:

	while (HeapTupleIsValid(tuple = systable_getnext(fkeyScan)))
	{
		Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple);

		/* Not referencing one of our list of tables */
		if (!list_member_oid(oids, con->confrelid))
			continue;

		/*
		 * If the constraint has a parent, climb up the partition hierarchy
		 * all the way to the top.  We need to process all the partitions
		 * covered by the topmost constraint.
		 */
		while (OidIsValid(con->conparentid))
		{
			scan2 = systable_beginscan(fkeyRel, ConstraintParentIndexId,
									   true, NULL, 1, &key);
			tup2 = heap_copytuple(systable_getnext(scan2));	/* XXX leaks memory */
			con = (Form_pg_constraint) GETSTRUCT(tup2);
			systable_endscan(scan2);
		}

		/* Add referencer to result, unless present in input list */
		if (!list_member_oid(relationIds, con->conrelid))
			result = lappend_oid(result, con->conrelid);
		if (conrelid is partitioned)
		{
			add each partition to result list;
		}
	}

ENOTIME to complete it now, though ... also: I'm not sure about having
heap_truncate_find_FKs() acquire the locks on partitions; but what
happens if there's a concurrent detach?

This is a larger can of worms than I imagined.  Maybe a simpler solution
is to say that you cannot truncate a partition; if you want that,
truncate the topmost relation.  No functionality seems lost with that
restriction, or is it?  And the semantics seem better defined anyway.
(AFAICS this is implemented easily: if we see a non-invalid conparentid,
raise an error).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services