On Sat, 2 Feb 2019 at 02:52, Robert Haas <robertmh...@gmail.com> wrote: > I think the key question here is whether or not you can cope with > someone having done arbitrary AEL-requiring modifications to the > delaylocked partitions. If you can, the fact that the plan might > sometimes be out-of-date is an inevitable consequence of doing this at > all, but I think we can argue that it's an acceptable consequence as > long as the resulting behavior is not too bletcherous. If you can't, > then this patch is dead.
I spent some time looking at this patch again and thinking about the locking. I ended up looking through each alter table subcommand by way of AlterTableGetLockLevel() and going through scenarios with each of them in my head to try to understand: a) If the subcommand can even be applied to a leaf partition; and b) Can the subcommand cause a cached plan to become invalid? I did all the easy ones first then started on the harder ones. I came to AT_DropConstraint and imagined the following scenario: -- ** session 1 create table listp (a int) partition by list(a); create table listp1 partition of listp for values in(1); create table listp2 partition of listp for values in(2); create index listp1_a_idx on listp1 (a); create index listp2_a_idx on listp2 (a); set enable_seqscan = off; set plan_cache_mode = force_generic_plan; prepare q1 (int) as select * from listp where a = $1; execute q1 (1); begin; execute q1 (1); -- ** session 2 drop index listp2_a_idx; -- ** session 1 execute q1 (2); ERROR: could not open relation with OID 16401 The only way I can think to fix this is to just never lock partitions at all, and if a lock is to be obtained on a partition, it must be instead obtained on the top-level partitioned table. That's a rather large change that could have large consequences, and I'm not even sure it's possible since we'd need to find the top-level parent before obtaining the lock, by then the hierarchy might have changed and we'd need to recheck, which seems like quite a lot of effort just to obtain a lock... Apart from that, it's not this patch, so looks like I'll need to withdraw this one :-( -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
