On Tue, Jan 19, 2021 at 2:03 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > >
> > > You have not raised a WARNING for the second case.
> >
> > The same checks in current Postgres code also don't raise a WARNING
> > for that case, so I'm just being consistent with existing Postgres
> > code (which itself isn't consistent for those two cases).
> >
>
> Search for the string "too few entries in indexprs list" and you will
> find a lot of places in code raising ERROR for the same condition.
>
Yes, but raising an ERROR stops processing (not just logs an error
message). Raising a WARNING logs a warning message and continues
processing. It's a big difference.
So, for the added parallel-safety-checking code, it was suggested by
Vignesh (and agreed by me) that, for these rare and highly unlikely
conditions, it would be best not to just copy the error-handling code
verbatim from other cases in the Postgres code (as I had originally
done) and just stop processing dead with an error, but to instead
return PARALLEL_UNSAFE, so that processing continues as it would for
current non-parallel processing, which would most likely error-out
anyway along the current error-handling checks and paths when those
bad attributes/fields are referenced.
I will add some Asserts() and don't mind adding a WARNING message for
the 2nd case.
If you really feel strongly about this, I can just restore the
original code, which will stop dead with an ERROR in the middle of
parallel-safety checking should one of these rare conditions ever
occur.
Regards,
Greg Nancarrow
Fujitsu Australia
