Re: [PATCH] Add PQconninfoParseParams and PQconninfodefaultsMerge to libpq

On 21.02.2013 16:09, Amit Kapila wrote:
> On Thursday, February 21, 2013 12:46 AM Heikki Linnakangas wrote:
>> The second adds it to
>> pg_dump, per above. The third adds it to pg_dumpall.
>>
>> The third patch is a bit complicated. It first parses the user-
>> specified connection string using PQconninfoParse, so that it can merge
>> in some extra keywords: user, host, password, dbname and
>> fallback_application_name. It then calls PQconnectdbParams with the
>> keyword/value pairs. After making the initial connection to postgres or
>> template1 database, it calls PQconninfo() to again extract the
>> keyword/value pairs in effect in the connection, and constructs a new
>> connection string from them. That new connection string is then passed
>> to pg_dump on the command line, with the database name appended to it.
>>
>> That seems to work, although it's perhaps a bit Rube Goldbergian. One
>> step of deparsing and parsing could be avoided by keeping the
>> keyword/value pairs from the first PQconninfoParse() call, instead of
>> constructing them again with PQconninfo(). I'll experiment with that
>> tomorrow.

I did the above, keeping the keyword/value pairs from the
PQconninfoParse() step instead.

> I think this whole new logic in pg_dumpall is needed because in
> pg_dump(Patch-2),
> we have used dbname for new option.
> In pg_dump, if for new option (-d) we use new variable conn_str similar as
> we used at other places and change
> the ConnectDatabase() in file pg_backup_db.c similar to GetConnection of
> pg_basebackup, we might not
> need the new logic of deparsing and parsing in pg_dumpall.

Yeah, could be. However, it's good to call the option -d for the sake of
consistency. Also, there would be some confusion if there were two ways
to pass a connection string (we'd stil have to support passing a
connection string as the database name, for backwards-compatibility).

I've committed those patches, with some further changes. If you have the
time, please take another look at the committed patches. Thanks!

- Heikki