Date: Thu, 17 Jan 2019 19:23:06 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	Jesper Pedersen <jesper.pedersen@redhat.com>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15587: Partitions with ALTER TABLE ADD CONSTRAINT
Message-ID: <201901172223.lm3iihkw7hn4@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="gr4h3ox77briqjuh"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <0757116d-f94e-ba78-b6d9-17ffcf8ff5bc@lab.ntt.co.jp>
User-Agent: NeoMutt/20180716-1013-2c1e88
Precedence: bulk


--gr4h3ox77briqjuh
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Jan-16, Amit Langote wrote:

> Why not just move the code in clone_fk_constraints() that checks if the
> constraint equivalent of the parent's constraint is present in the
> partition and simply attach the two without creating a new copy for the
> partition to a new function in tablecmds.c and call the function from both
> clone_fk_constraints() and ATAddForeignKeyConstraint()?  Attached is what
> I'm thinking.

Well, the whole point of my proposal is that the FK-creating code
recurses to partitions by calling ATAddForeignKeyConstraint, and IMO
that's the wrong level to recurse at; we should instead recurse by
calling clone_fk_constraints() as a whole.  That's not readibly possible
with the current code arrangement because of layering, but after
backpatching (as attached) the two patches I mentioned, I end up with
the following, which I think is much cleaner.  (Also, this code layout
plays much better with my project to continue to extend FKs so that they
are allowed to point to partitioned tables; see the other thread).

The attached patches are for pg11; they don't apply to master.  The
changes are uninteresting.

The tests added by the final commit clearly show dupe FKs being created
in some of the cases, if you run them without applying the code fixes,
and none afterwards.

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

--gr4h3ox77briqjuh
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Refactor-duplicate-code-into-DeconstructFkConstraint.patch"