From f2d4a284f30cdab8cbdde6803876a0f5f3b8951e Mon Sep 17 00:00:00 2001 From: David Christensen Date: Mon, 18 May 2026 19:26:39 +0000 Subject: [PATCH 1/3] pgduck client: include conninfo in start-failure error Add an errdetail() clause to the "could not start query engine" ereport() so users can see which connection string was being used when the connection to pgduck_server failed. Issue: #293 Signed-off-by: David Christensen --- pg_lake_engine/src/pgduck/client.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pg_lake_engine/src/pgduck/client.c b/pg_lake_engine/src/pgduck/client.c index e64e6c32..63054d3b 100644 --- a/pg_lake_engine/src/pgduck/client.c +++ b/pg_lake_engine/src/pgduck/client.c @@ -144,10 +144,14 @@ GetPGDuckConnection(void) PQfinish(connection); #ifdef USE_ASSERT_CHECKING - ereport(ERROR, (errmsg("could not start query engine: %s", errorMessage))); + ereport(ERROR, + (errmsg("could not start query engine: %s", errorMessage), + errdetail("connection string: %s", PgduckServerConninfo))); #else /* hide internals from users */ - ereport(ERROR, (errmsg("could not start query engine"))); + ereport(ERROR, + (errmsg("could not start query engine"), + errdetail("connection string: %s", PgduckServerConninfo))); #endif } From 085dd9b69417fc1deb7512f4d7515a56f669402f Mon Sep 17 00:00:00 2001 From: David Christensen Date: Fri, 22 May 2026 21:23:01 +0000 Subject: [PATCH 2/3] pgduck client: log conninfo to server log instead of errdetail Per review on PR #361: the connection string may include credentials and is an internal detail not appropriate for the client. Emit a separate LOG ereport() with errmsg + errdetail before the user-facing ERROR so administrators can still debug misconfigured servers (issue #293) via the PostgreSQL server log without leaking the conninfo to the client. Signed-off-by: David Christensen --- pg_lake_engine/src/pgduck/client.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pg_lake_engine/src/pgduck/client.c b/pg_lake_engine/src/pgduck/client.c index 63054d3b..f43a5be5 100644 --- a/pg_lake_engine/src/pgduck/client.c +++ b/pg_lake_engine/src/pgduck/client.c @@ -143,15 +143,22 @@ GetPGDuckConnection(void) PQfinish(connection); -#ifdef USE_ASSERT_CHECKING - ereport(ERROR, - (errmsg("could not start query engine: %s", errorMessage), + /* + * Log the conninfo to the server log so an administrator can debug + * misconfigured servers (issue #293) without exposing the connection + * string -- which may include credentials -- to the client. We + * intentionally do not include the libpq error message here; it is + * surfaced only in the assertion-enabled ERROR below. + */ + ereport(LOG_SERVER_ONLY, + (errmsg("could not start query engine"), errdetail("connection string: %s", PgduckServerConninfo))); + +#ifdef USE_ASSERT_CHECKING + ereport(ERROR, (errmsg("could not start query engine: %s", errorMessage))); #else /* hide internals from users */ - ereport(ERROR, - (errmsg("could not start query engine"), - errdetail("connection string: %s", PgduckServerConninfo))); + ereport(ERROR, (errmsg("could not start query engine"))); #endif } From bba35ff2df2f44f2717b3c953d7af660de9c8e1b Mon Sep 17 00:00:00 2001 From: David Christensen Date: Wed, 27 May 2026 16:36:09 +0000 Subject: [PATCH 3/3] pgduck client: resolve env-supplied conninfo for log Add ResolvePgduckConninfo(), which uses PQconnectStart + PQconninfo to let libpq overlay environment variables (PGHOSTADDR, PGPORT, etc.) and compiled-in defaults onto pg_lake_engine.host, then formats the result as a conninfo string with passwords and debug-only fields stripped. The LOG_SERVER_ONLY errdetail emitted on connection failure now uses this resolved string, so an administrator debugging issue #293-style env-var overrides sees the values libpq actually used rather than just the configured GUC. Signed-off-by: David Christensen --- pg_lake_engine/src/pgduck/client.c | 77 +++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/pg_lake_engine/src/pgduck/client.c b/pg_lake_engine/src/pgduck/client.c index f43a5be5..14925cba 100644 --- a/pg_lake_engine/src/pgduck/client.c +++ b/pg_lake_engine/src/pgduck/client.c @@ -39,6 +39,7 @@ static void InitializePGDuckClient(void); static void SetupPgDuckConnectionHash(void); +static char *ResolvePgduckConninfo(void); static void PGDuckClientTransactionCallback(XactEvent event, void *arg); static void PGDuckClientSubtransactionCallback(SubXactEvent event, @@ -127,6 +128,77 @@ SetupPgDuckConnectionHash(void) } +/* + * ResolvePgduckConninfo returns a palloc'd connection string showing the + * options libpq would actually use to reach pgduck_server, including values + * supplied via environment variables (e.g. PGHOSTADDR, PGPORT) and libpq's + * compiled-in defaults. This is what an administrator needs to debug a + * misconfigured server -- PgduckServerConninfo alone hides any env-supplied + * overrides. + * + * We resolve options without opening a socket: PQconndefaults() returns the + * options with environment variables and compiled-in defaults applied, and + * PQconninfoParse() returns the values from the configured conninfo string. + * Configured values override defaults. Options whose dispchar starts with + * '*' (passwords) or 'D' (debug-only) are omitted, as are empty values. + */ +static char * +ResolvePgduckConninfo(void) +{ + StringInfoData buf; + + initStringInfo(&buf); + + PQconninfoOption *defaults = PQconndefaults(); + PQconninfoOption *configured = PQconninfoParse(PgduckServerConninfo, NULL); + + if (defaults == NULL) + { + /* OOM inside libpq; fall back to the raw configured string */ + if (configured != NULL) + PQconninfoFree(configured); + appendStringInfoString(&buf, PgduckServerConninfo); + return buf.data; + } + + bool first = true; + + for (PQconninfoOption *opt = defaults; opt->keyword != NULL; opt++) + { + const char *val = opt->val; + + if (configured != NULL) + { + for (PQconninfoOption *c = configured; c->keyword != NULL; c++) + { + if (strcmp(c->keyword, opt->keyword) == 0) + { + if (c->val != NULL) + val = c->val; + break; + } + } + } + + if (val == NULL || val[0] == '\0') + continue; + if (opt->dispchar != NULL && + (opt->dispchar[0] == '*' || opt->dispchar[0] == 'D')) + continue; + + appendStringInfo(&buf, "%s%s='%s'", + first ? "" : " ", opt->keyword, val); + first = false; + } + + PQconninfoFree(defaults); + if (configured != NULL) + PQconninfoFree(configured); + + return buf.data; +} + + /* * GetPGDuckConnection returns the main PGDuck connection. */ @@ -150,9 +222,12 @@ GetPGDuckConnection(void) * intentionally do not include the libpq error message here; it is * surfaced only in the assertion-enabled ERROR below. */ + char *resolved = ResolvePgduckConninfo(); + ereport(LOG_SERVER_ONLY, (errmsg("could not start query engine"), - errdetail("connection string: %s", PgduckServerConninfo))); + errdetail("connection string: %s", resolved))); + pfree(resolved); #ifdef USE_ASSERT_CHECKING ereport(ERROR, (errmsg("could not start query engine: %s", errorMessage)));