On Sat, Sep 8, 2018 at 3:57 AM Chris Travers <chris.travers@adjust.com> wrote:
> Attached is the patch we are fully testing at Adjust.
Thanks!
> I have run make check on Linux and MacOS, and make check-world on Linux (check-world fails on MacOS on all versions
andall branches due to ecpg failures).
FWIW it's entirely possible to get make check-world passing on a Mac.
Maybe post the problem you're seeing to a new thread?
> ...
> In the past it had been suggested we do PG_TRY(); and PG_CATCH(), but given that it is not consistent whether we can
raisean error or whether we MUST raise an error, I don't see how this approach can work. As far as I can see, we MUST
raisean error in the appropriate spot if and only if elevel is set to a sufficient level.
Yeah, your way looks a bit nicer than something involving PG_TRY().
> Is there any feedback on this approach before I add it to the next commitfest?
Please go ahead and add it. Being a bug fix, we'll commit it sooner
than the open commitfest anyway, but it's useful to have it in there.
+ if (errno == EINTR && elevel >= ERROR)
+ CHECK_FOR_INTERRUPTS();
I think we might as well just CHECK_FOR_INTERRUPTS() unconditionally.
In this branch elevel is always ERROR as you noted, and the code
around there is confusing enough already.
+ } while (rc == EINTR && !(ProcDiePending || QueryCancelPending));
There seems to be a precedent for checking QueryCancelPending directly
to break out of a loop in regcomp.c and syncrep.c. So this seems OK.
Hmm, I wonder if there should be an INTERRUPT_PENDING() macro that
hides those details, but allows you to break out of a loop and then do
some cleanup before CHECK_FOR_INTERRUPT().
--
Thomas Munro
http://www.enterprisedb.com