RE: [bug?] Missed parallel safety checks, and wrong parallel safety

On Wednesday, June 16, 2021 11:27 AM houzj.fnst@fujitsu.com wrote:
> On Tuesday, June 15, 2021 10:01 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > Just to be clear here, I don't think it really matters what we *want*
> > to do. I don't think it's reasonably *possible* to check all the
> > partitions, because we don't hold locks on them. When we're assessing
> > a bunch of stuff related to an individual relation, we have a lock on
> > it. I think - though we should double-check tablecmds.c - that this is
> > enough to prevent all of the dependent objects - triggers,
> > constraints, etc. - from changing. So the stuff we care about is
> > stable. But the situation with a partitioned table is different. In
> > that case, we can't even examine that stuff without locking all the
> > partitions. And even if we do lock all the partitions, the stuff could change
> immediately afterward and we wouldn't know. So I think it would be difficult to
> >make it correct.
> > Now, maybe it could be done, and I think that's worth a little more
> > thought. For example, perhaps whenever we invalidate a relation, we
> > could also somehow send some new, special kind of invalidation for its
> > parent saying, essentially, "hey, one of your children has changed in
> > a way you might care about." But that's not good enough, because it
> > only goes up one level. The grandparent would still be unaware that a
> > change it potentially cares about has occurred someplace down in the
> > partitioning hierarchy. That seems hard to patch up, again because of
> > the locking rules. The child can know the OID of its parent without
> > locking the parent, but it can't know the OID of its grandparent
> > without locking its parent. Walking up the whole partitioning
> > hierarchy might be an issue for a number of reasons, including
> > possible deadlocks, and possible race conditions where we don't emit
> > all of the right invalidations in the face of concurrent changes. So I
> > don't quite see a way around this part of the problem, but I may well be
> missing something. In fact I hope I am missing something, because solving this
> problem would be really nice.
> 
> I think the check of partition could be even more complicated if we need to
> check the parallel safety of partition key expression. If user directly insert into a
> partition, then we need invoke ExecPartitionCheck which will execute all it's
> parent's and grandparent's partition key expressions. It means if we change a
> parent table's partition key expression(by 1) change function in expr or 2)
> attach the parent table as partition of another parent table), then we need to
> invalidate all its child's relcache.
> 
> BTW, currently, If user attach a partitioned table 'A' to be partition of another
> partitioned table 'B', the child of 'A' will not be invalidated.

To be honest, I didn't find a cheap way to invalidate partitioned table's
parallel safety automatically. For one thing, We need to recurse higher
in the partition tree to invalid all the parent table's relcache(and perhaps
all its children's relcache) not only when alter function parallel safety,
but also for DDLs which will invalid the partition's relcache, such as
CREATE/DROP INDEX/TRIGGER/CONSTRAINT. It seems too expensive. For another,
even if we can invalidate the partitioned table's parallel safety
automatically, we still need to lock all the partition when checking table's
parallel safety, because the partition's parallel safety could be changed
after checking the parallel safety.

So, IMO, at least for partitioned table, an explicit flag looks more acceptable.
For regular table, It seems we can work it out automatically, although 
I am not sure does anyone think it looks a bit inconsistent.

Best regards,
houzj