Hi,
2012-11-21 19:19 keltezéssel, Magnus Hagander írta:
I'm breaking this out into it's own thread, for my own sanity if
nothing else :) And it's an isolated feature after all.
I still agree with the previous review at
http://archives.postgresql.org/message-id/1349321071.23971.0.ca...@vanquo.pezone.net
about keeping the data in more than one place.
OK, it seems I completely missed that comment.
(Or forgot about it if I happened to answer it.)
Based on that, I've created a different version of this patch,
attached. This way we keep all the data in one struct.
I like this single structure but not the way you handle the
options' classes. In your patch, each option belongs to only
one class. These classes are:
PG_CONNINFO_REPLICATION = "replication" only
PG_CONNINFO_PASSWORD = "password" only
PG_CONNINFO_NORMAL = everything else
How does it help pg_basebackup to filter out e.g. dbname and replication?
These are added by the walreceiver module anyway and adding them
to the primary_conninfo line should even be discouraged by the documentation.
In my view, the classes should be inclusive:
PG_CONNINFO_NORMAL = Everything that's usable for a regular client
connection. This mean everything, maybe including "password" but
excluding "replication".
PG_CONNINFO_PASSWORD = "password" only.
PG_CONNINFO_REPLICATION = Everything usable for a replication client
not added by walreceiver. Maybe including/excluding "password".
Maybe there should be two flags for replication usage:
PG_CONNINFO_WALRECEIVER = everything except those not added
by walreceiver (and maybe "password" too)
PG_CONNINFO_REPLICATION = "replication" only
And every option can belong to more than one class, just as in my patch.
At this point, the patch is untested beyond compiling and a trivial
psql check, because I ran out of time :) But I figured I'd throw it
out there for comments on which version people prefer. (And yes, it's
quite possible I've made a big think-mistake in it somewhere, but
again, better to get some eyes on it early)
My version also contains a fixed version of the docs that should be
moved back into Zoltans version if that's the one we end up
preferring.
I also liked your version of the documentation better,
I am not too good at writing docs.
Also, a question was buried in the other review which is - are we OK
to remove the requiressl parameter. Both these patches do so, because
the code becomes much simpler if we can do that. It has been
deprecated since 7.2. Is it OK to remove it, or do we need to put back
in the more complex code to deal with both?
Attached is both Zoltans latest patch (v16) and my slightly different approach.
Comments on which approach is best?
Test results from somebody who has the time to look at it? :)
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Thanks for four work on this.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
