[PATCH] Add support for displaying database service in psql prompt
Hi,
I think it might be useful to sometimes display the database service
(i.e. what is defined in ~/.pg_service.conf and used via psql
service=foo) in the psql prompt, e.g. if you have 'test' and 'prod'
services, but both have the same database name. This was also suggested
to me during a recent customer training.
I chose the '%s' tag for it. I had to add the service to PGConn as
PQservice (first patch) to libpq and then use it in psql in the second
patch.
Michael
Attachments:
v1-0001-Add-PQservice-to-PGConn.patchtext/x-diff; charset=us-asciiDownload
From f876195acb797a5ac58c17409fdd75d18581c292 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:27:52 +0100
Subject: [PATCH v1 1/2] Add PQservice to PGConn.
This adds the content of the database service (if any) to PGConn. One
use for this would be for psql to display the service as part of the
prompt.
---
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 11 ++++++++++-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 5d8213e0b5..2ad2cbf5ca 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
+PQservice 206
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 61c025ff3b..8d809ee4cb 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -190,7 +190,8 @@ typedef struct _internalPQconninfoOption
static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
- "Database-Service", "", 20, -1},
+ "Database-Service", "", 20,
+ offsetof(struct pg_conn, pgservice)},
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
@@ -7052,6 +7053,14 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
+char *
+PQservice(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ return conn->pgservice;
+}
+
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 15012c770c..5947e7c766 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -385,6 +385,7 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
+extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 08cc391cbd..dcebca9898 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -394,6 +394,7 @@ struct pg_conn
char *fbappname; /* fallback application name */
char *dbName; /* database name */
char *replication; /* connect as the replication standby? */
+ char *pgservice; /* Postgres service, if any */
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
--
2.39.5
v1-0002-Add-support-for-database-service-to-psql-prompt.patchtext/x-diff; charset=us-asciiDownload
From 3257e93d20353ff348b15df9b45207ec45839ed5 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:49:14 +0100
Subject: [PATCH v1 2/2] Add support for database service to psql prompt.
This adds a new psql variable SERVICE as well as the string '%s' for the
psql PROMPT, displaying the service (from PGSERVICEFILE) if so desired.
---
src/bin/psql/command.c | 2 ++
src/bin/psql/prompt.c | 6 ++++++
2 files changed, 8 insertions(+)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 328d78c73f..e8f2573649 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4085,6 +4085,7 @@ SyncVariables(void)
pset.sversion = PQserverVersion(pset.db);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
+ SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
@@ -4118,6 +4119,7 @@ void
UnsyncVariables(void)
{
SetVariable(pset.vars, "DBNAME", NULL);
+ SetVariable(pset.vars, "SERVICE", NULL);
SetVariable(pset.vars, "USER", NULL);
SetVariable(pset.vars, "HOST", NULL);
SetVariable(pset.vars, "PORT", NULL);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 0d99d00ac9..de3d976103 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -33,6 +33,7 @@
* %p - backend pid
* %> - database server port number
* %n - database user name
+ * %s - service
* %/ - current database
* %~ - like %/ but "~" when database name equals user name
* %w - whitespace of the same width as the most recent output of PROMPT1
@@ -246,6 +247,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
}
break;
+ case 's':
+ if (PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
+
case 'l':
snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
break;
--
2.39.5
Compiled and tested: works fine, so +1 from me. Honestly, I was surprised
%s was still available. :)
Cheers,
Greg
Hi,
On Tue, Nov 19, 2024 at 07:47:58PM -0500, Greg Sabino Mullane wrote:
Compiled and tested: works fine, so +1 from me. Honestly, I was surprised
%s was still available. :)
Thanks. Was that full review? You kept the commitfest item in "Needs
Review" state.
Michael
On Thu, Nov 28, 2024 at 09:17:23PM +0100, Michael Banck wrote:
Hi,
On Tue, Nov 19, 2024 at 07:47:58PM -0500, Greg Sabino Mullane wrote:
Compiled and tested: works fine, so +1 from me. Honestly, I was surprised
%s was still available. :)Thanks. Was that full review? You kept the commitfest item in "Needs
Review" state.
No objections to this proposal, that can be useful. No objections
with the use of 's' for the shortcut in the psql prompt.
Back to your patch, you are missing two things at quick glance:
- Documentation for the new libpq API in 0001.
- Documentation for the new SERVICE and its new %s (see section called
"Prompting" on the psql page.
So please make sure to provide these with the next version of the
patch.
--
Michael
Hi Michael,
On Tue, Dec 10, 2024 at 04:38:24PM +0900, Michael Paquier wrote:
On Thu, Nov 28, 2024 at 09:17:23PM +0100, Michael Banck wrote:
On Tue, Nov 19, 2024 at 07:47:58PM -0500, Greg Sabino Mullane wrote:
Compiled and tested: works fine, so +1 from me. Honestly, I was surprised
%s was still available. :)Thanks. Was that full review? You kept the commitfest item in "Needs
Review" state.No objections to this proposal, that can be useful. No objections
with the use of 's' for the shortcut in the psql prompt.
Cool.
Back to your patch, you are missing two things at quick glance:
- Documentation for the new libpq API in 0001.
- Documentation for the new SERVICE and its new %s (see section called
"Prompting" on the psql page.So please make sure to provide these with the next version of the
patch.
Thanks, I have added the documentation now in v2.
Michael
Attachments:
v2-0001-Add-PQservice-to-PGConn.patchtext/x-diff; charset=us-asciiDownload
From 666b80297f1cb918230b5104e2e8dce08b711394 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:27:52 +0100
Subject: [PATCH v2 1/2] Add PQservice to PGConn.
This adds the content of the database service (if any) to PGConn. One
use for this would be for psql to display the service as part of the
prompt. It also adds PGservice() as a new connection status unction.
---
doc/src/sgml/libpq.sgml | 20 ++++++++++++++++++++
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 11 ++++++++++-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
5 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 01f259fd0d..105b22b317 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2530,6 +2530,26 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQservice">
+ <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Returns the service of the active connection.
+
+<synopsis>
+char *PQservice(const PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
+ <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
+ Otherwise, if there was no service provided, it returns an empty string.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 5d8213e0b5..2ad2cbf5ca 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
+PQservice 206
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aaf87e8e88..ddcc7b60ab 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -190,7 +190,8 @@ typedef struct _internalPQconninfoOption
static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
- "Database-Service", "", 20, -1},
+ "Database-Service", "", 20,
+ offsetof(struct pg_conn, pgservice)},
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
@@ -7040,6 +7041,14 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
+char *
+PQservice(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ return conn->pgservice;
+}
+
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 15012c770c..5947e7c766 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -385,6 +385,7 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
+extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 08cc391cbd..dcebca9898 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -394,6 +394,7 @@ struct pg_conn
char *fbappname; /* fallback application name */
char *dbName; /* database name */
char *replication; /* connect as the replication standby? */
+ char *pgservice; /* Postgres service, if any */
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
--
2.39.5
v2-0002-Add-support-for-database-service-to-psql-prompt.patchtext/x-diff; charset=us-asciiDownload
From 5faa624871e3267a780d794e9ad2e9f42fd42311 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:49:14 +0100
Subject: [PATCH v2 2/2] Add support for database service to psql prompt.
This adds a new psql variable SERVICE as well as the string '%s' for the
psql PROMPT, displaying the service (from PGSERVICEFILE) if so desired.
---
doc/src/sgml/ref/psql-ref.sgml | 14 ++++++++++++++
src/bin/psql/command.c | 2 ++
src/bin/psql/prompt.c | 6 ++++++
3 files changed, 22 insertions(+)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e42073ed74..af78e0e87b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4380,6 +4380,15 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-service">
+ <term><varname>SERVICE</varname></term>
+ <listitem>
+ <para>
+ The service from <filename>pg_service.conf</filename>, if applicable.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-shell-error">
<term><varname>SHELL_ERROR</varname></term>
<listitem>
@@ -4685,6 +4694,11 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
(tilde) if the database is your default database.</para></listitem>
</varlistentry>
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry, if any.</para></listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-prompting-numbersign">
<term><literal>%#</literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 1f3cbb11f7..cd16f27947 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4082,6 +4082,7 @@ SyncVariables(void)
pset.sversion = PQserverVersion(pset.db);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
+ SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
@@ -4115,6 +4116,7 @@ void
UnsyncVariables(void)
{
SetVariable(pset.vars, "DBNAME", NULL);
+ SetVariable(pset.vars, "SERVICE", NULL);
SetVariable(pset.vars, "USER", NULL);
SetVariable(pset.vars, "HOST", NULL);
SetVariable(pset.vars, "PORT", NULL);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 0d99d00ac9..de3d976103 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -33,6 +33,7 @@
* %p - backend pid
* %> - database server port number
* %n - database user name
+ * %s - service
* %/ - current database
* %~ - like %/ but "~" when database name equals user name
* %w - whitespace of the same width as the most recent output of PROMPT1
@@ -246,6 +247,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
}
break;
+ case 's':
+ if (PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
+
case 'l':
snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
break;
--
2.39.5
On Mon, Dec 16, 2024 at 10:57:49PM +0100, Michael Banck wrote:
Thanks, I have added the documentation now in v2.
The doc additions seem fine to me. I've just grabbed three tiny nits,
nothing critical.
+ case 's':
+ if (PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
Other code paths of get_prompt check for pset.db being NULL. True
that it does not matter when calling PQservice() with a connection
that does not exist. For consistency with the surroundings this
should be done at least?
+ <para>
+ The service from <filename>pg_service.conf</filename>, if applicable.
+ </para>
pg_service.conf is not especially true as it depends on the
environment used. For psql, perhaps just use "The service name, if
applicable". No need to be fancy.
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
[...]
+PQservice 206
You didn't miss that, nice.
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry, if any.</para></listitem>
+ </varlistentry>
Other entries don't use "if any", would just cut it.
--
Michael
Hi,
On Tue, Dec 17, 2024 at 04:43:21PM +0900, Michael Paquier wrote:
On Mon, Dec 16, 2024 at 10:57:49PM +0100, Michael Banck wrote:
Thanks, I have added the documentation now in v2.
The doc additions seem fine to me. I've just grabbed three tiny nits,
nothing critical.
Thanks for the further review.
+ case 's': + if (PQservice(pset.db)) + strlcpy(buf, PQservice(pset.db), sizeof(buf)); + break;Other code paths of get_prompt check for pset.db being NULL. True
that it does not matter when calling PQservice() with a connection
that does not exist. For consistency with the surroundings this
should be done at least?
Ok, I've done that.
+ <para> + The service from <filename>pg_service.conf</filename>, if applicable. + </para>pg_service.conf is not especially true as it depends on the
environment used. For psql, perhaps just use "The service name, if
applicable". No need to be fancy.
Done.
+ <varlistentry id="app-psql-prompting-s"> + <term><literal>%s</literal></term> + <listitem><para>The name of the service entry, if any.</para></listitem> + </varlistentry>Other entries don't use "if any", would just cut it.
Done.
V3 attached.
Michael
Attachments:
v3-0001-Add-PQservice-to-PGConn.patchtext/x-diff; charset=us-asciiDownload
From 1e71982edce8744f48906d2857d22727691b7267 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:27:52 +0100
Subject: [PATCH v3 1/2] Add PQservice to PGConn.
This adds the content of the database service (if any) to PGConn. One
use for this would be for psql to display the service as part of the
prompt. It also adds PGservice() as a new connection status unction.
---
doc/src/sgml/libpq.sgml | 20 ++++++++++++++++++++
src/interfaces/libpq/exports.txt | 1 +
src/interfaces/libpq/fe-connect.c | 11 ++++++++++-
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 1 +
5 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 01f259fd0d..105b22b317 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2530,6 +2530,26 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
+ <varlistentry id="libpq-PQservice">
+ <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
+
+ <listitem>
+ <para>
+ Returns the service of the active connection.
+
+<synopsis>
+char *PQservice(const PGconn *conn);
+</synopsis>
+ </para>
+
+ <para>
+ <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
+ <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
+ Otherwise, if there was no service provided, it returns an empty string.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 5d8213e0b5..2ad2cbf5ca 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,3 +205,4 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
+PQservice 206
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index aaf87e8e88..ddcc7b60ab 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -190,7 +190,8 @@ typedef struct _internalPQconninfoOption
static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
- "Database-Service", "", 20, -1},
+ "Database-Service", "", 20,
+ offsetof(struct pg_conn, pgservice)},
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
@@ -7040,6 +7041,14 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
+char *
+PQservice(const PGconn *conn)
+{
+ if (!conn)
+ return NULL;
+ return conn->pgservice;
+}
+
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 15012c770c..5947e7c766 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -385,6 +385,7 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
+extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 08cc391cbd..dcebca9898 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -394,6 +394,7 @@ struct pg_conn
char *fbappname; /* fallback application name */
char *dbName; /* database name */
char *replication; /* connect as the replication standby? */
+ char *pgservice; /* Postgres service, if any */
char *pguser; /* Postgres username and password, if any */
char *pgpass;
char *pgpassfile; /* path to a file containing password(s) */
--
2.39.5
v3-0002-Add-support-for-database-service-to-psql-prompt.patchtext/x-diff; charset=us-asciiDownload
From f70fc781adf0833ecd148d672f22d6a1d860c365 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Thu, 31 Oct 2024 18:49:14 +0100
Subject: [PATCH v3 2/2] Add support for database service to psql prompt.
This adds a new psql variable SERVICE as well as the string '%s' for the
psql PROMPT, displaying the service (from PGSERVICEFILE) if so desired.
---
doc/src/sgml/ref/psql-ref.sgml | 14 ++++++++++++++
src/bin/psql/command.c | 2 ++
src/bin/psql/prompt.c | 6 ++++++
3 files changed, 22 insertions(+)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e42073ed74..6ba85d9acf 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -4380,6 +4380,15 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-service">
+ <term><varname>SERVICE</varname></term>
+ <listitem>
+ <para>
+ The service name, if applicable.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-shell-error">
<term><varname>SHELL_ERROR</varname></term>
<listitem>
@@ -4685,6 +4694,11 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
(tilde) if the database is your default database.</para></listitem>
</varlistentry>
+ <varlistentry id="app-psql-prompting-s">
+ <term><literal>%s</literal></term>
+ <listitem><para>The name of the service entry.</para></listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-prompting-numbersign">
<term><literal>%#</literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 1f3cbb11f7..cd16f27947 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4082,6 +4082,7 @@ SyncVariables(void)
pset.sversion = PQserverVersion(pset.db);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
+ SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
@@ -4115,6 +4116,7 @@ void
UnsyncVariables(void)
{
SetVariable(pset.vars, "DBNAME", NULL);
+ SetVariable(pset.vars, "SERVICE", NULL);
SetVariable(pset.vars, "USER", NULL);
SetVariable(pset.vars, "HOST", NULL);
SetVariable(pset.vars, "PORT", NULL);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 0d99d00ac9..3b3f079229 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -33,6 +33,7 @@
* %p - backend pid
* %> - database server port number
* %n - database user name
+ * %s - service
* %/ - current database
* %~ - like %/ but "~" when database name equals user name
* %w - whitespace of the same width as the most recent output of PROMPT1
@@ -246,6 +247,11 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
}
break;
+ case 's':
+ if (pset.db && PQservice(pset.db))
+ strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ break;
+
case 'l':
snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
break;
--
2.39.5
On Tue, Dec 17, 2024 at 09:42:36AM +0100, Michael Banck wrote:
Done.
V3 attached.
Done.
--
Michael
Hi,
On Wed, Dec 18, 2024 at 03:17:36PM +0900, Michael Paquier wrote:
On Tue, Dec 17, 2024 at 09:42:36AM +0100, Michael Banck wrote:
Done.
V3 attached.
Done.
Thanks!
Michael
On Wed, Dec 18, 2024 at 03:17:36PM +0900, Michael Paquier wrote:
Done.
This new PQservice() function from commit 4b99fed75 came up in the annual
exports.txt diff. The standard in libpq has been to not clutter the API with
new functions that simply retrieve one PQconninfoOption value. PQconninfo()
provides access to all those values in a generic way. What do you think of
making psql use PQconninfo() for this, then removing PQservice()? The rest of
the commit (adding the struct field, necessary for PQconninfo() to include the
value) looks good.
On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote:
This new PQservice() function from commit 4b99fed75 came up in the annual
exports.txt diff. The standard in libpq has been to not clutter the API with
new functions that simply retrieve one PQconninfoOption value. PQconninfo()
provides access to all those values in a generic way. What do you think of
making psql use PQconninfo() for this, then removing PQservice()? The rest of
the commit (adding the struct field, necessary for PQconninfo() to include the
value) looks good.
Sure, I was not aware of such a policy. Relying on PQconninfoOption
is less efficient because we would need to look through the whole set
of options when looking for the service name, and this needs one extra
allocation as PQconninfoFree() frees the values allocated. With two
callers perhaps this inefficiency is OK to live with anyway.
What do you think about the attached, then?
--
Michael
Attachments:
pqservice-removal.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9fcd2db83265..c839634d4233 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4480,6 +4480,7 @@ SyncVariables(void)
{
char vbuf[32];
const char *server_version;
+ char *service_name;
/* get stuff from connection */
pset.encoding = PQclientEncoding(pset.db);
@@ -4489,12 +4490,16 @@ SyncVariables(void)
setFmtEncoding(pset.encoding);
SetVariable(pset.vars, "DBNAME", PQdb(pset.db));
- SetVariable(pset.vars, "SERVICE", PQservice(pset.db));
SetVariable(pset.vars, "USER", PQuser(pset.db));
SetVariable(pset.vars, "HOST", PQhost(pset.db));
SetVariable(pset.vars, "PORT", PQport(pset.db));
SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding));
+ service_name = get_service_name();
+ SetVariable(pset.vars, "SERVICE", service_name);
+ if (service_name)
+ pg_free(service_name);
+
/* this bit should match connection_warnings(): */
/* Try to get full text form of version, might include "devel" etc */
server_version = PQparameterStatus(pset.db, "server_version");
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index d2c0a49c46c0..3593c0468310 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -2531,6 +2531,41 @@ session_username(void)
return PQuser(pset.db);
}
+/*
+ * Return the service name of the current connection.
+ *
+ * The caller is responsible for freeing the result value allocated.
+ */
+char *
+get_service_name(void)
+{
+ PQconninfoOption *opts;
+ PQconninfoOption *serviceopt = NULL;
+ char *res = NULL;
+
+ if (pset.db == NULL)
+ return NULL;
+
+ opts = PQconninfo(pset.db);
+ if (opts == NULL)
+ return NULL;
+
+ for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
+ {
+ if (strcmp(opt->keyword, "service") == 0)
+ {
+ serviceopt = opt;
+ continue;
+ }
+ }
+
+ /* Take a copy of the value, as it is freed by PQconninfoFree(). */
+ if (serviceopt && serviceopt->val != NULL)
+ res = pg_strdup(serviceopt->val);
+ PQconninfoFree(opts);
+
+ return res;
+}
/* expand_tilde
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 7f1a23de1e82..261199df3187 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -39,6 +39,7 @@ extern bool SendQuery(const char *query);
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
+extern char *get_service_name(void);
extern void expand_tilde(char **filename);
extern void clean_extended_state(void);
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 3aa7d2d06c80..cdae9b681500 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -169,8 +169,15 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
break;
/* service name */
case 's':
- if (pset.db && PQservice(pset.db))
- strlcpy(buf, PQservice(pset.db), sizeof(buf));
+ {
+ char *service_name = get_service_name();
+
+ if (service_name)
+ {
+ strlcpy(buf, service_name, sizeof(buf));
+ pg_free(service_name);
+ }
+ }
break;
/* backend pid */
case 'p':
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 0625cf39e9af..dbbae642d769 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -205,9 +205,8 @@ PQcancelFinish 202
PQsocketPoll 203
PQsetChunkedRowsMode 204
PQgetCurrentTimeUSec 205
-PQservice 206
-PQsetAuthDataHook 207
-PQgetAuthDataHook 208
-PQdefaultAuthDataHook 209
-PQfullProtocolVersion 210
-appendPQExpBufferVA 211
+PQsetAuthDataHook 206
+PQgetAuthDataHook 207
+PQdefaultAuthDataHook 208
+PQfullProtocolVersion 209
+appendPQExpBufferVA 210
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 51a9c4165845..09eb79812ac6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7461,14 +7461,6 @@ PQdb(const PGconn *conn)
return conn->dbName;
}
-char *
-PQservice(const PGconn *conn)
-{
- if (!conn)
- return NULL;
- return conn->pgservice;
-}
-
char *
PQuser(const PGconn *conn)
{
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 7d3a9df6fd55..af8004f952a5 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -400,7 +400,6 @@ extern int PQrequestCancel(PGconn *conn);
/* Accessor functions for PGconn objects */
extern char *PQdb(const PGconn *conn);
-extern char *PQservice(const PGconn *conn);
extern char *PQuser(const PGconn *conn);
extern char *PQpass(const PGconn *conn);
extern char *PQhost(const PGconn *conn);
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 298c4b38ef90..b2c2cf9eac83 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2740,26 +2740,6 @@ char *PQport(const PGconn *conn);
</listitem>
</varlistentry>
- <varlistentry id="libpq-PQservice">
- <term><function>PQservice</function><indexterm><primary>PQservice</primary></indexterm></term>
-
- <listitem>
- <para>
- Returns the service of the active connection.
-
-<synopsis>
-char *PQservice(const PGconn *conn);
-</synopsis>
- </para>
-
- <para>
- <xref linkend="libpq-PQservice"/> returns <symbol>NULL</symbol> if the
- <parameter>conn</parameter> argument is <symbol>NULL</symbol>.
- Otherwise, if there was no service provided, it returns an empty string.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry id="libpq-PQtty">
<term><function>PQtty</function><indexterm><primary>PQtty</primary></indexterm></term>
On Mon, Jul 07, 2025 at 11:06:06AM +0900, Michael Paquier wrote:
On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote:
This new PQservice() function from commit 4b99fed75 came up in the annual
exports.txt diff. The standard in libpq has been to not clutter the API with
new functions that simply retrieve one PQconninfoOption value. PQconninfo()
provides access to all those values in a generic way. What do you think of
making psql use PQconninfo() for this, then removing PQservice()? The rest of
the commit (adding the struct field, necessary for PQconninfo() to include the
value) looks good.Sure, I was not aware of such a policy. Relying on PQconninfoOption
is less efficient because we would need to look through the whole set
of options when looking for the service name, and this needs one extra
allocation as PQconninfoFree() frees the values allocated. With two
callers perhaps this inefficiency is OK to live with anyway.
I think the choice to make there is whether to call PQconninfo() once per
prompt emission or to cache the value, invalidating that cache e.g. once per
SyncVariables(). My first thought was to cache, but the decision is not too
important. A PQconninfo() call is likely negligible relative to all that
happens between prompts. Even if not negligible, the overhead of not caching
will affect only prompts using the new escape sequence.
On Sun, Jul 06, 2025 at 08:00:09PM -0700, Noah Misch wrote:
I think the choice to make there is whether to call PQconninfo() once per
prompt emission or to cache the value, invalidating that cache e.g. once per
SyncVariables(). My first thought was to cache, but the decision is not too
important. A PQconninfo() call is likely negligible relative to all that
happens between prompts. Even if not negligible, the overhead of not caching
will affect only prompts using the new escape sequence.
SyncVariables() happens at startup and when re-syncing a connection
during a check, so that does not really worry me.
By the way, there is a second change in the CF that's suggesting the
addition of a SERVICEFILE variable, which also uses a separate libpq
API (forgot about this one):
https://commitfest.postgresql.org/patch/5387/
Changing the patch on the other thread to use a conninfo is
stright-forward. How about extending the get_service_name()@common.c
I've proposed upthread so as it takes a string in input and it could
be reused for more connection options than only "service" so as it
could be reused there as well?
--
Michael
On Tue, Jul 08, 2025 at 08:52:08AM +0900, Michael Paquier wrote:
On Sun, Jul 06, 2025 at 08:00:09PM -0700, Noah Misch wrote:
I think the choice to make there is whether to call PQconninfo() once per
prompt emission or to cache the value, invalidating that cache e.g. once per
SyncVariables(). My first thought was to cache, but the decision is not too
important. A PQconninfo() call is likely negligible relative to all that
happens between prompts. Even if not negligible, the overhead of not caching
will affect only prompts using the new escape sequence.SyncVariables() happens at startup and when re-syncing a connection
during a check, so that does not really worry me.By the way, there is a second change in the CF that's suggesting the
addition of a SERVICEFILE variable, which also uses a separate libpq
API (forgot about this one):
https://commitfest.postgresql.org/patch/5387/Changing the patch on the other thread to use a conninfo is
stright-forward. How about extending the get_service_name()@common.c
I've proposed upthread so as it takes a string in input and it could
be reused for more connection options than only "service" so as it
could be reused there as well?
I'd prefer not to get involved in decisions affecting only psql efficiency and
psql code cosmetics. Please make that decision without my input.
On Tue, Jul 08, 2025 at 05:41:32PM -0700, Noah Misch wrote:
I'd prefer not to get involved in decisions affecting only psql efficiency and
psql code cosmetics. Please make that decision without my input.
Okay, I have used this more extensible routine then, planning to use
it for the other patch. The prompt shortcut retrieves the value using
a GetVariable(), rather than looking at the connection options all the
time.
--
Michael