On 13-03-20 05:54 PM, Tom Lane wrote:
Steve Singer <ssin...@ca.afilias.info> writes:
From a end-user expectations point of view I am okay with somehow
marking the structure returned by PQconndefaults in a way that the
connect calls will later fail.
Unless the program changes the value of PGSERVICE, surely all subsequent
connection attempts will fail for the same reason, regardless of what
PQconndefaults returns?
regards, tom lane
So your proposing we do something like the attached patch? Where we
change conninfo_add_defaults to ignore an invalid PGSERVICE if being
called by PQconndefaults() but keep the existing behaviour in other
contexts where it is actually being used to establish a connection?
In this case even if someone takes the result of PQconndefaults and uses
that to build connection options for a new connection it should fail
when it does the pgservice lookup when establishing the connection.
That sounds reasonable to me.
Steve
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
index ed67759..c1d8d69 100644
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** check_pghost_envvar(void)
*** 300,306 ****
/* Get valid libpq env vars from the PQconndefaults function */
start = PQconndefaults();
!
for (option = start; option->keyword != NULL; option++)
{
if (option->envvar && (strcmp(option->envvar, "PGHOST") == 0 ||
--- 300,309 ----
/* Get valid libpq env vars from the PQconndefaults function */
start = PQconndefaults();
! if (start == NULL)
! {
! pg_log(PG_FATAL,"can not get default connection options out of memory\n");
! }
for (option = start; option->keyword != NULL; option++)
{
if (option->envvar && (strcmp(option->envvar, "PGHOST") == 0 ||
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index eea9c6b..0a9a29e 100644
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** static PQconninfoOption *conninfo_array_
*** 347,353 ****
const char *const * values, PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
static bool conninfo_add_defaults(PQconninfoOption *options,
! PQExpBuffer errorMessage);
static PQconninfoOption *conninfo_uri_parse(const char *uri,
PQExpBuffer errorMessage, bool use_defaults);
static bool conninfo_uri_parse_options(PQconninfoOption *options,
--- 347,354 ----
const char *const * values, PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
static bool conninfo_add_defaults(PQconninfoOption *options,
! PQExpBuffer errorMessage,
! bool ignore_missing_service);
static PQconninfoOption *conninfo_uri_parse(const char *uri,
PQExpBuffer errorMessage, bool use_defaults);
static bool conninfo_uri_parse_options(PQconninfoOption *options,
*************** PQconndefaults(void)
*** 875,881 ****
connOptions = conninfo_init(&errorBuf);
if (connOptions != NULL)
{
! if (!conninfo_add_defaults(connOptions, &errorBuf))
{
PQconninfoFree(connOptions);
connOptions = NULL;
--- 876,882 ----
connOptions = conninfo_init(&errorBuf);
if (connOptions != NULL)
{
! if (!conninfo_add_defaults(connOptions, &errorBuf,true))
{
PQconninfoFree(connOptions);
connOptions = NULL;
*************** conninfo_parse(const char *conninfo, PQE
*** 4243,4249 ****
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
--- 4244,4250 ----
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage,false))
{
PQconninfoFree(options);
return NULL;
*************** conninfo_array_parse(const char *const *
*** 4395,4401 ****
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
--- 4396,4402 ----
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage,false))
{
PQconninfoFree(options);
return NULL;
*************** conninfo_array_parse(const char *const *
*** 4416,4422 ****
* error condition here --- we just leave the option's value as NULL.
*/
static bool
! conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
{
PQconninfoOption *option;
char *tmp;
--- 4417,4424 ----
* error condition here --- we just leave the option's value as NULL.
*/
static bool
! conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage,
! bool ignore_missing_service)
{
PQconninfoOption *option;
char *tmp;
*************** conninfo_add_defaults(PQconninfoOption *
*** 4425,4431 ****
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters.
*/
! if (parseServiceInfo(options, errorMessage) != 0)
return false;
/*
--- 4427,4434 ----
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters.
*/
! if (parseServiceInfo(options, errorMessage) != 0 &&
! ! ignore_missing_service)
return false;
/*
*************** conninfo_uri_parse(const char *uri, PQEx
*** 4511,4517 ****
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
--- 4514,4520 ----
*/
if (use_defaults)
{
! if (!conninfo_add_defaults(options, errorMessage,false))
{
PQconninfoFree(options);
return NULL;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
