Re: Parallel INSERT SELECT take 2

On 8/4/26 05:45, Zhijie Hou (Fujitsu) wrote:
> On Monday, May 11, 2026 7:44 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>> On 5/11/26 09:21, Zhijie Hou (Fujitsu) wrote:
>>> On Saturday, May 9, 2026 5:29 PM Tomas Vondra <tomas(at)vondra(dot)me>
>> wrote:
>>>> ...
>>> After searching my memory and reviewing the old discussion, I recall
>>> another locking-related issue...
>>>
>>> For example, consider creating a new table that uses a function in an
>>> index expression (or altering a table to add a new expression or
>>> function). If a user tries to alter the function used in that
>>> expression concurrently, the ALTER FUNCTION command cannot see the
>> newly created table because it hasn't been committed yet.
>>> As a result, we cannot ensure that pg_class.parallel_safety is updated
>>> in a concurrency-safe manner. The cause is that we don't hold a lock
>>> on the function when creating or altering a table.
>> ...
>>
>> Yes, problems like this may be tricky.
>>
>> But I don't see a problem with requiring an exclusive lock on a function when
>> changing the parallel safety for a function (and updating the pg_class attribute
>> for all relations that use it). Yes, it's not great, it'd be nice to do it with weaker
>> locks, but if we can't ... sorry.
>
> Per an off-list discussion, I'm sharing an alternative approach for
> consideration. The basic idea is to cache parallel safety in the relcache and
> invalidate the cached value whenever: a parallel-safety-relevant object (e.g.,
> trigger, index, constraint) is added to or dropped from a table, or a function's
> parallel-safety flag is altered. This approach essentially follows the design
> from the earlier thread [1], but remove the parlalel safety declaration and
> extends it slightly by also attempting to compute safety for partitioned tables.
>

Thanks for the patch.

> The detailed design is as follows:
>
> Modifying a table while in parallel mode is only safe if none of the objects
> attached to the table involves parallel-unsafe or parallel-restricted functions:
> triggers, index expressions and predicates, CHECK constraints, column default
> expressions, and the partition key. For a partitioned table, each partition is
> checked recursively as well.
>

Is this aiming to allow the insert to be parallel too, or just the
select part? If only the select part is parallel, wouldn't it be OK to
have parallel-restricted expressions?

> Checking all of that for every query is expensive, so cache the result in the
> relation's relcache entry: rd_paralleldml holds the worst hazard level found
> (one of the PROPARALLEL_xxx values), or zero if it has not been computed yet.
> The new RelationGetParallelDmlSafety() function computes the value on first use
> and returns the cached value thereafter.
>
> The cached value is invalidated whenever a function's parallel-safety flag is
> altered, or whenever a parallel-safety-relevant object is added to or dropped
> from the table or, for a partitioned table, from any of its partitions.
>

OK

> When a function's parallel safety changes, we invalidate the cached
> parallel-safety flag in all relcache entries, rather than introducing heavier
> locking or reverse-engineering the set of tables that reference the function.
> Function-safety changes are expected to be rare, so this broad invalidation
> should be acceptable.
>

OK, as you say, it should be a rare event. And it only resets the flag,
keeping the rest of the relcache data.

> Because we do not lock the function while altering its safety, a race is
> possible: the safety flag could change after another backend has already used
> the cached value to build its plan. This is no worse than current HEAD behavior,
> since a function has always been free to be altered without blocking concurrent
> DML.
>
So the user can execute a long-running DML, and while it's running
someone could alter all kinds of function parameters, including the
parallel safety. But the DML will keep running.

I guess that's probably OK, or rather not a new issue.

The next execution should use the correct "new" value, right? Or could
it happen that we miss an invalidation? Let's say we start building the
relcache entry, when a function gets altered. Could it happen that we
end up with a "stale" value of the flag in relcache until the end of the
session?

> When a partition's parallel safety changes, we invalidate the cached values of
> all its ancestors in the partition tree. We deliberately do not take locks on
> the ancestors: this avoids introducing new deadlock risk, and it avoids changing
> locking behavior in a way that would make commands suddenly block normal DML
> that previously ran unimpeded - a change that might be hard for users to reason
> about. Find the ancestors without locking is safe because a partition cannot be
> concurrently attached or detached - both ATTACH PARTITION and DETACH PARTITION
> lock the child tables during DDL execution. The only remaining race is that an
> unsafe object could be added to a partition while a parallel INSERT ... SELECT
> is already executing; in that case, execution detects the hazard and raises an
> ERROR before inserting into that partition. This rare, detectable execution-time
> error is the trade-off for keeping the locking scheme simple.
>

Hmm, OK. We need to document this argument somewhere (maybe it's in a
comment already, not sure). Would it be possible to have a TAP test
(with injection points?) / isolation test for this?

> Because the cached value lives in relcache, every new session must recompute it
> on first use. This is true of all relcache data, but it is slightly more costly
> here since the safety computation can be noticeable for tables with many
> partitions. A possible future improvement is a fixed-size shared hash table
> storing parallel-safety values, so that only the first session to touch a table
> pays the computation cost while later sessions can reuse the result.
>

This is my main concern - that we'll have to do this (possibly fairly
expensive) check, even for short inserts that can't possibly benefit
from parallelism. What's the worst case impact? Say I have a workload
that runs small inserts into a partitioned table, each in a new
connection. How expensive would that be?

(I agree opening a new connection for each insert seems silly, it's
merely a synthetic worst-case scenario.)

> Just share this approach for discussing, if this approach and the old
> approach[1] both turn out to be unacceptable, we could switch to Tomas's
> approach of materialize the SELECT result. The attachment implements this
> approach for reference.
>

+1 to using the spilling approach as a fall back, in case this turns out
not to work.

I wonder if we might want to do both. The spilling approach does not
even need to check the parallel safety - it can parallelize even cases
where the insert has parallel unsafe expressions, etc. I wonder how many
cases would benefit from that.

> To provide context for the old approach for comparison, here is a brief overview
> of how it works. It is a hybrid approach: we allow the user to explicitly
> specify the parallel safety of a table via a new DDL command, ALTER TABLE ...
> PARALLEL SAFETY. If the user does not specify it, the behavior depends on the
> table type: for non-partitioned tables, we automatically compute the safety and
> cache it in relcache; for partitioned tables, we default to disallowing parallel
> SELECT. To enable parallel INSERT ... SELECT on a partitioned table, the user
> must set the safety manually. When executing ALTER TABLE ... PARALLEL SAFETY, we
> validate whether the table actually matches the specified safety level and raise
> an ERROR if it does not.
>

FWIW I agree we really don't want to require users to specify parallel
safety for tables - doing that manually would be very tedious, error
prone (how would the user even know with tables to modify?). Unlike for
functions, the database is able that for tables.

My main question is whether it's OK to store that just in the relcache,
and why it's safer/better than actually keeping it in pg_class. I mean,
we could reset that just like the relcache, the first session would
update it with the fresh value, etc.

regards

--
Tomas Vondra