Re: fork/exec patch - Mailing list pgsql-patches

From Claudio Natoli
Subject Re: fork/exec patch
Date
Msg-id A02DEC4D1073D611BAE8525405FCCE2B028096@harris.memetrics.local
Whole thread Raw
In response to fork/exec patch  (Claudio Natoli <claudio.natoli@memetrics.com>)
List pgsql-patches
Hi Neil,


> > No. This isn't necessary (and what action would it take in any
> > case?).
>
> It should write a log message. I'm not sure why this /shouldn't/ be
> done: if an operation fails, we should log that failure. This is
> standard practise.

Fair enough. Will do (although, I'd point out that there are more than a few
places in the existing code where unlink is called without error checking,
but that isn't justification for not doing it here).


> >> Doesn't this function still acquire ShmemIndexLock? (i.e. why was
> >> this comment changed?)
> >
> > AFAICS this is just whitespace differences.
>
> Read it again. Here's the whole diff hunk:
> [snip]
> The code acquires ShmemIndexLock, performs some computations, and then
> notes that "ShmemLock is held" in a comment before returning. ISTM
> that is plainly wrong.

[I did, again. Twice just now. And still didn't see what you were trying to
point out. And then...]

Ah. Yep. Typo. Will fix (I was experimenting with using the ShmemLock,
instead of creating a new ShmemIndexLock, and forgot to change the comment
back).


> > [ the only other suggested changes are ] stylistic/cosmetic and
> > effect the EXEC_BACKEND code only.
>
> Yeah, my apologies for nitpicking...

Not at all. I want this done well. Thank you for any input.

Cheers,
Claudio

---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

pgsql-patches by date:

Previous
From: "Peter Eisentraut"
Date:
Subject: Re: Double Backslash example patch
Next
From: Tom Lane
Date:
Subject: Re: fork/exec patch