On Wed, Jul 31, 2019 at 02:02:03PM +0800, Ning Yu wrote:
> Yes, in current postgres source code there are several wrappers of
> mkdir() that do similar jobs. If we could have a safe mkdir_p()
> implementation then we could use it directly in all these wrappers, that
> could save a lot of maintenance effort in the long run. I'm not saying
> that our patches are enough to make it safe and reliable, and I agree
> that any patches may introduce new bugs, but I think that having a safe
> and unified mkdir_p() is a good direction to go.
That's my impression as well. Please note that this does not involve
an actual bug in Postgres and that this is rather invasive, so this
does not really qualify for a back-patch. No objections to simplify
the backend on HEAD though. It would be good if you could actually
register a patch to the commit fest app [1] and also rework the patch
set so as at least PathNameCreateTemporaryDir wins its simplifications
for the first problem (double-checking the other code paths would be
nice as well). The EEXIST handling, and the confusion about EEXIST
showing for both a path and a file need some separate handling (not
sure what to do on these parts yet).
> Yes, that's why we put the testing module in a separate patch from the
> fix, feel free to ignore it. In fact ourselves have concerns about it ;)
I think that it is nice that you took the time to do so as you get
yourself more familiar with the TAP infrastructure in the tree and
prove your point. For this case, I would not have gone to do this
much though ;p
[1]: https://commitfest.postgresql.org/24/
--
Michael