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 <alvhe...@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
