On Sat, Sep 8, 2018 at 3:57 AM Chris Travers <chris.trav...@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 and all 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 raise an 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 raise an 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
