On 8/4/26 05:45, Zhijie Hou (Fujitsu) wrote: > On Monday, May 11, 2026 7:44 PM Tomas Vondra <[email protected]> wrote: >> On 5/11/26 09:21, Zhijie Hou (Fujitsu) wrote: >>> On Saturday, May 9, 2026 5:29 PM Tomas Vondra <[email protected]> >> 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
