From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: postgres_fdw hint messages
Date: Thu, 25 Aug 2022 15:42:40 +0200
Message-ID: <b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com> (raw)
The postgres_fdw tests contain this (as amended by patch 0001):
ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
ERROR: invalid option "password"
HINT: Valid options in this context are: service, passfile,
channel_binding, connect_timeout, dbname, host, hostaddr, port, options,
application_name, keepalives, keepalives_idle, keepalives_interval,
keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert,
sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer,
ssl_min_protocol_version, ssl_max_protocol_version, gssencmode,
krbsrvname, gsslib, target_session_attrs, use_remote_estimate,
fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable,
fetch_size, batch_size, async_capable, parallel_commit, keep_connections
This annoys developers who are working on libpq connection options,
because any option added, removed, or changed causes this test to need
to be updated.
It's also questionable how useful this hint is in its current form,
considering how long it is and that the options are in an
implementation-dependent order.
Possible changes:
- Hide the hint from this particular test (done in the attached patches).
- Keep the hint, but maybe make it sorted?
- Remove all the hints like this from foreign data commands.
- Don't show the hint when there are more than N valid options.
- Do some kind of "did you mean" like we have for column names.
Thoughts?
Attachments:download all (2 files, .tar.gz)0001-postgres_fdw-Remove-useless-DO-block-in-test.patch (text/plain, 2.7K) download
From 4adb3b6846e98a55ee19d8425dfb3d6d12145f16 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 25 Aug 2022 15:31:10 +0200
Subject: [PATCH 1/2] postgres_fdw: Remove useless DO block in test
---
contrib/postgres_fdw/expected/postgres_fdw.out | 8 +-------
contrib/postgres_fdw/sql/postgres_fdw.sql | 6 +-----
2 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex 7bf35602b0..a42c9720c3 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -9621,15 +9621,9 @@ ERROR: password is required
DETAIL: Non-superusers must provide a password in the user mapping.
-- If we add a password to the connstr it'll fail, because we don't allow passwords
-- in connstrs only in user mappings.
-DO $d$- BEGIN- EXECUTE $$ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')$$;- END;-$d$;+ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
ERROR: invalid option "password"
HINT: Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable, fetch_size, batch_size, async_capable, parallel_commit, keep_connections
-CONTEXT: SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"-PL/pgSQL function inline_code_block line 3 at EXECUTE
-- If we add a password for our user mapping instead, we should get a different
-- error because the password wasn't actually *used* when we run with trust auth.
--
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sqlindex 42735ae78a..63581a457d 100644--- a/contrib/postgres_fdw/sql/postgres_fdw.sql+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql@@ -2886,11 +2886,7 @@ CREATE FOREIGN TABLE pg_temp.ft1_nopw (
-- If we add a password to the connstr it'll fail, because we don't allow passwords
-- in connstrs only in user mappings.
-DO $d$- BEGIN- EXECUTE $$ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')$$;- END;-$d$;+ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
-- If we add a password for our user mapping instead, we should get a different
-- error because the password wasn't actually *used* when we run with trust auth.
--
2.37.1
0002-postgres_fdw-Avoid-listing-all-libpq-connection-opti.patch (text/plain, 2.8K) download
From 986ea253cd887b472e8040580f8c1a2642f3cada Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 25 Aug 2022 15:31:11 +0200
Subject: [PATCH 2/2] postgres_fdw: Avoid listing all libpq connection options
in error hint
---
contrib/postgres_fdw/expected/postgres_fdw.out | 7 ++++++-
contrib/postgres_fdw/sql/postgres_fdw.sql | 6 ++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex a42c9720c3..3678428b95 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -9621,9 +9621,14 @@ ERROR: password is required
DETAIL: Non-superusers must provide a password in the user mapping.
-- If we add a password to the connstr it'll fail, because we don't allow passwords
-- in connstrs only in user mappings.
+--+-- Set to terse verbosity to avoid hint listing all possible libpq+-- connection options, which causes annoying test failures when doing+-- libpq work.+\set VERBOSITY terse
ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
ERROR: invalid option "password"
-HINT: Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, sslcrldir, sslsni, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, truncatable, fetch_size, batch_size, async_capable, parallel_commit, keep_connections+\set VERBOSITY default
-- If we add a password for our user mapping instead, we should get a different
-- error because the password wasn't actually *used* when we run with trust auth.
--
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sqlindex 63581a457d..a8b65cdc88 100644--- a/contrib/postgres_fdw/sql/postgres_fdw.sql+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql@@ -2885,8 +2885,14 @@ CREATE FOREIGN TABLE pg_temp.ft1_nopw (
-- If we add a password to the connstr it'll fail, because we don't allow passwords
-- in connstrs only in user mappings.
+--+-- Set to terse verbosity to avoid hint listing all possible libpq+-- connection options, which causes annoying test failures when doing+-- libpq work.+\set VERBOSITY terse
ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw');
+\set VERBOSITY default
-- If we add a password for our user mapping instead, we should get a different
-- error because the password wasn't actually *used* when we run with trust auth.
--
2.37.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@enterprisedb.com
Subject: Re: postgres_fdw hint messages
In-Reply-To: <b1f9f399-3a1a-b554-283f-4ae7f34608e2@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox