Hi Amit, Greg, Sorry, I hadn't noticed last week that some questions were addressed to me.
On Sat, Mar 6, 2021 at 7:19 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > Thanks, your changes look good to me. I went ahead and changed the > patch to track the partitionOids once rather than in two separate > lists and use that list in PlanCacheRelCallback to invalidate the > plans. Not mixing the partition OIDs into relationOids seems fine to me. I had considered that option too, but somehow forgot to mention it here. A couple of things that look odd in v24-0001 (sorry if there were like that from the beginning): +static bool +target_rel_max_parallel_hazard(max_parallel_hazard_context *context) +{ + bool max_hazard_found; + + Relation targetRel; + + /* + * The target table is already locked by the caller (this is done in the + * parse/analyze phase), and remains locked until end-of-transaction. + */ + targetRel = table_open(context->target_rte->relid, + context->target_rte->rellockmode); The comment seems to imply that the lock need not be retaken here, but the code does. Maybe it's fine to pass NoLock here, but use rellockmode when locking partitions, because they would not have been locked by the parser/analyzer. Which brings me to: +static bool +target_rel_partitions_max_parallel_hazard(Relation rel, + max_parallel_hazard_context *context) +{ ... + for (i = 0; i < pdesc->nparts; i++) + { + bool max_hazard_found; + Relation part_rel; + + /* + * The partition needs to be locked, and remain locked until + * end-of-transaction to ensure its parallel-safety state is not + * hereafter altered. + */ + part_rel = table_open(pdesc->oids[i], AccessShareLock); Not that I can prove there to be any actual hazard, but we tend to avoid taking locks with different strengths in the same query as would occur with this piece of code. We're locking the partition with AccessShareLock here, but the executor will lock the partition with the stronger RowExclusiveLock mode before trying to insert into it. Better to use the stronger mode to begin with or just use the target partitioned table's RTE's rellockmode which would be RowExclusiveLock. You can see that that's what AcquireExecutorLocksOnPartitions() will do in the generic plan case. -- Amit Langote EDB: http://www.enterprisedb.com