From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: getting ERROR "relation 16401 has no triggers" with partition foreign key alter |
Date: | 2019-07-22 22:18:29 |
Message-ID: | 20190722221829.GA30093@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2019-Jul-17, Alvaro Herrera wrote:
> On 2019-Jul-17, Alvaro Herrera wrote:
> > I wonder if there are other AT subcommands that are similarly broken,
> > because many of them skip the CheckTableNotInUse for the partitions.
>
> I suppose the question here is where else do we need to call the new
> ATRecurseCheckNotInUse function (which needs a comment).
I decided to rename the new function to ATCheckPartitionsNotInUse, and
make it a no-op for legacy inheritance. This seems quite specific to
partitioned tables (as opposed to legacy inheritance behavior).
After looking at the code some more, I think calling the new function in
the Prep phase is correct. The attached patch is pretty much final form
for this bugfix. I decided to unwrap a couple of error messages (I did
get bitten while grepping because of this), and reordered one of the new
Identity command cases in ATPrepCmd since it appeared in inconsistent
order in that one place of four.
I looked at all the other AT subcommand cases that might require the
same treatment, and didn't find anything -- either the recursion is done
at Prep time, which checks already, or contains the proper check at Exec
time right after opening the partition rel. (I think it would be better
to do the check during the Prep phase, to avoid wasting work in case a
partition happens to be used. However, that's not critical and not for
this commit to fix IMO.)
Separately from that, there's AT_SetLogged / AT_SetUnlogged which look
pretty dubious ... I'm not sure that recursion is handled correctly
there. Maybe it's considered okay to have a partitioned table with
unlogged partitions, and vice versa?
I also noticed that AT_AlterConstraint does not handle recursion at all,
and it also has this comment:
* Currently only works for Foreign Key constraints.
* Foreign keys do not inherit, so we purposely ignore the
* recursion bit here, but we keep the API the same for when
* other constraint types are supported.
which sounds to oppose reality.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Check-partitions-not-in-use.patch | text/x-diff | 4.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jonathan S. Katz | 2019-07-22 22:48:11 | Re: initdb recommendations |
Previous Message | Fabien COELHO | 2019-07-22 22:18:19 | Re: Add CREATE DATABASE LOCALE option |