Re: Parallel INSERT (INTO ... SELECT ...)

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(dot)kapila16(at)gmail(dot)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