On 2020-Sep-23, Amit Langote wrote:
Hi Amit, thanks for reviewing this patch!
> On Tue, Aug 4, 2020 at 8:49 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> I suspect that we don't really need this defensive constraint. I mean
> even after committing the 1st transaction, the partition being
> detached still has relispartition set to true and
> RelationGetPartitionQual() still returns the partition constraint, so
> it seems the constraint being added above is duplicative.
Ah, thanks for thinking through that. I had vague thoughts about this
being unnecessary in the current mechanics, but hadn't fully
materialized the thought. (The patch was completely different a few
unposted iterations ago).
> Moreover, the constraint is not removed as part of any cleaning up
> after the DETACH process, so repeated attach/detach of the same
> partition results in the constraints piling up:
Yeah, I knew about this issue (mentioned in my self-reply on Aug 4) and
didn't worry too much about it because I was thinking I'd rather get rid
of the constraint addition in the first place.
> Noticed a bug/typo in the patched RelationBuildPartitionDesc():
>
> - inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock);
> + inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock,
> + include_detached);
>
> You're passing NoLock for include_detached which means you never
> actually end up including detached partitions from here.
I fixed this in the version I posted on Sept 10. I think you were
reading the version posted at the start of this thread.
Thanks,
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
