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

On Mon, Jun 14, 2021 at 9:08 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Jun 14, 2021 at 2:32 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > Why do you think we don't need to check index AM functions? Say we
> > have an index expression that uses function and if its parallel safety
> > is changed then probably that also impacts whether we can do insert in
> > parallel. Because otherwise, we will end up executing some parallel
> > unsafe function in parallel mode during index insertion.
>
> I'm not saying that we don't need to check index expressions. I agree
> that we need to check those. The index AM functions are things like
> btint4cmp(). I don't think that a function like that should ever be
> parallel-unsafe.
>

Okay, but I think if we go with your suggested model where whenever
there is a change in parallel-safety of any function, we need to send
the new invalidation then I think it won't matter whether the function
is associated with index expression, check constraint in the table, or
is used in any other way.

>
> Even if this line of thinking is correct, there's a big issue for
> partitioning hierarchies because there you need to know stuff about
> relations that you don't have any other reason to open. I'm just
> arguing that if there's no partitioning, the problem seems reasonably
> solvable. Either you changed something about the relation, in which
> case you've got to lock it and issue invalidations, or you've changed
> something about the function, which could be handled via a new type of
> invalidation. I don't really see why the cost would be particularly
> bad. Suppose that for every relation, you have a flag which is either
> PARALLEL_DML_SAFE, PARALLEL_DML_RESTRICTED, PARALLEL_DML_UNSAFE, or
> PARALLEL_DML_SAFETY_UNKNOWN. When someone sends a message saying "some
> existing function's parallel-safety changed!" you reset that flag for
> every relation in the relcache to PARALLEL_DML_SAFETY_UNKNOWN. Then if
> somebody does DML on that relation and we want to consider
> parallelism, it's got to recompute that flag. None of that sounds
> horribly expensive.
>

Sounds reasonable. I will think more on this and see if anything else
comes to mind apart from what you have mentioned.

> I mean, it could be somewhat annoying if you have 100k relations open
> and sit around all day flipping parallel-safety markings on and off
> and then doing a single-row insert after each flip, but if that's the
> only scenario where we incur significant extra overhead from this kind
> of design, it seems clearly better than forcing users to set a flag
> manually. Maybe it isn't, but I don't really see what the other
> problem would be right now. Except, of course, for partitioning, which
> I'm not quite sure what to do about.
>

Yeah, dealing with partitioned tables is tricky. I think if we don't
want to check upfront the parallel safety of all the partitions then
the other option as discussed could be to ask the user to specify the
parallel safety of partitioned tables. We can additionally check the
parallel safety of partitions when we are trying to insert into a
particular partition and error out if we detect any parallel-unsafe
clause and we are in parallel-mode. So, in this case, we won't be
completely relying on the users. Users can either change the parallel
safe option of the table or remove/change the parallel-unsafe clause
after error. The new invalidation message as we are discussing would
invalidate the parallel-safety for individual partitions but not the
root partition (partitioned table itself). For root partition, we will
rely on information specified by the user.

I am not sure if we have a simple way to check the parallel safety of
partitioned tables. In some way, we need to rely on user either (a) by
providing an option to specify whether parallel Inserts (and/or other
DMLs) can be performed, or (b) by providing a guc and/or rel option
which indicate that we can check the parallel-safety of all the
partitions. Yet another option that I don't like could be to
parallelize inserts on non-partitioned tables.

--
With Regards,
Amit Kapila.