Message-ID: <6761397c.df0a0220.68885.210d@mx.google.com>
Sender: Michael Banck <mbanck@gmail.com>
Date: Tue, 17 Dec 2024 09:42:36 +0100
From: Michael Banck <mbanck@gmx.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: Greg Sabino Mullane <htamfids@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Add support for displaying database service in psql
 prompt
References: <6723c612.050a0220.1567f4.b94a@mx.google.com>
 <CAKAnmmKhnHGp4X3FvMAicRsbyiKTWwMJ6oWdkRw2oLVzdeL-0A@mail.gmail.com>
 <6748cfd4.050a0220.1537dc.43bb@mx.google.com>
 <Z1fv8D_t1ok8svgO@paquier.xyz>
 <6760a25e.a70a0220.2e9d8f.d542@mx.google.com>
 <Z2ErmZD1lImYt_as@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="Kj7319i9nmIyA2yE"
Content-Disposition: inline
In-Reply-To: <Z2ErmZD1lImYt_as@paquier.xyz>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: <https://www.postgresql.org/message-id/6761397c.df0a0220.68885.210d%40mx.google.com>
Precedence: bulk


--Kj7319i9nmIyA2yE
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--Kj7319i9nmIyA2yE
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v3-0001-Add-PQservice-to-PGConn.patch"

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


--Kj7319i9nmIyA2yE
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v3-0002-Add-support-for-database-service-to-psql-prompt.patch"

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=&gt; <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


--Kj7319i9nmIyA2yE--