Re: [PATCHES] COPY when 'filename' is a directory

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I should have said "in the _new_ code" above.

Ah, my mistake. I was looking at the original coding.

> Agreed. However, I have never seen it stated that file descriptors are
> freeded on elog(ERROR). I certainly didn't know that. If we are going
> to remove them, let's do it systematically. Let's state in the
> developers FAQ what elog(ERROR) frees automatically (file descriptors,
> memory in most contexts, anything else?) and then I can check all the
> elog(ERROR) cases and make sure this is done consistently.

As a rule of thumb, *every* transient resource should be freed
automatically on elog(ERROR); if it isn't, what happens if there's an
elog in a called subroutine? If manual releasing of resources in an
error path is actually necessary, that should be regarded as an unsafe
coding practice, IMHO.

There are a small number of places that avoid this problem by not
calling any subroutines at all while holding a resource; the main
example I can think of in 7.2 is spinlocks, which don't have any
auto-release facility because they're not supposed to be held over
any nontrivial span of code anyway. But for most sorts of resources
I'd consider that approach unacceptably fragile.

Again, early release of an LWLock before elog is OK --- it's not
necessary, but there might be a performance justification for it.
But I don't think that argument applies to open files or memory
or other resources used within a single backend. There are or
should be auto-release-on-error mechanisms for all that stuff.

regards, tom lane

PS: The difference between AllocateFile and plain fopen() is exactly
that AllocateFile ensures the file will be closed on elog ...