Hi,
On 2021-03-24 14:42:44 +1100, Greg Nancarrow wrote:
> On Wed, Mar 24, 2021 at 5:44 AM Andres Freund <andres@anarazel.de> wrote:
> > Shouldn't we have this in IndexOptInfo already?
>
> The additional parallel-safety checks are (at least currently) invoked
> as part of max_parallel_hazard(), which is called early on in the
> planner, so I don't believe that IndexOptInfo/RelOptInfo has been
> built at this point.
Then that's something you need to redesign, not duplicate the effort.
> > But also, why on earth
> > is that WARNING branch a good idea?
> I think there are about half a dozen other places in the Postgres code
> where the same check is done, in which case it calls elog(ERROR,...).
> Is it a better strategy to immediately exit the backend with an error
> in this case, like the other cases do?
Yes.
> My take on it was that if this "should never happen" condition is ever
> encountered, let it back out of the parallel-safety checks and
> error-out in the place it normally (currently) would.
What advantage does that have? You'll get a bunch of WARNINGs before the
ERROR later in optimize, differences between assert-non/assert builds,
more complicated code flow, longer untested code.
Greetings,
Andres Freund