On 07/02/2012 09:45 PM, Tom Lane wrote:
> Honza Horak <hhorak@redhat.com> writes:
>> On 06/15/2012 05:40 PM, Honza Horak wrote:
>>> I realized the patch has some difficulties -- namely the socket path in the data dir lock file, which currently
usesone port for socket and the same for interface. So to allow users to use arbitrary port for all unix sockets, we'd
needto add another line only for unix socket, which doesn't apply for other platforms. Or we could just say that the
firstsocket will allways use the default port (PostPortNumber), which is a solution I prefer currently, but will be
gladfor any other opinion. This is also why there is still un-necesary string splitting in pg_ctl.c, which will be
removedafter the issue above is solved.
>
> I did a review pass over this patch.
>
I have finally an enhanced patch, see the attachment and feel free to
comment.
> Well, not so much "forbids" as "silently ignores", which doesn't seem like
> great user-interface design to me. If we're going to adopt this solution
> I think we need it to throw an error instead of just ignoring the port
> specification.
Alternate-port-number support has been removed from the patch, as per
Tom's e-mail from 07/03/12. It can be add in the future, if we really
need it.
> * I'm not especially thrilled with propagating SplitUnixDirectories calls
> into those two places anyway, nor with the weird decision for
> SplitUnixDirectories to return a separate "mainSocket" value. Perhaps
> what would be most sensible is to attach an assign hook to the
> unix_socket_directories GUC parameter that would automatically split the
> string and store the components into a globally-visible List variable
> (which could replace the globally-visible string value we have now).
Replacing the old global string value would probably need a new
configuration type "List" to be added, since otherwise guc works with it
as with a string. Adding that seems like too big overhead to me and thus
it seems better to add a new global (List *) variable and let the
original value of type (char *) to store non-parsed value.
Except that I believe all other Tom's comments have been involved.
Regards,
Honza