Hi
so 4. 2. 2023 v 21:36 odesÃlatel Corey Huinker <corey.huin...@gmail.com>
napsal:
> with doc and unsetting variable
>>
>> Regards
>>
>> Pavel
>>
>>
> Patch applies.
>
> Manually testing confirms that it works, at least for the connected state.
> I don't actually know how get psql to invoke DISCONNECT, so I killed the
> dev server and can confirm
>
> [222281:14:57:01 EST] corey=# \echo :BACKEND_PID
> 222281
> [222281:14:57:04 EST] corey=# select 1;
> FATAL: terminating connection due to administrator command
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
> Time: 1.554 ms
> [:15:02:31 EST] !> \echo :BACKEND_PID
> :BACKEND_PID
>
> Clearly, it is hard to write a regression test for an externally volatile
> value. `SELECT sign(:BACKEND_PID)` would technically do the job, if we're
> striving for completeness.
>
I did simple test - :BACKEND_PID should be equal pg_backend_pid()
>
> The inability to easily DISCONNECT via psql, and the deleterious effect
> that would have on other regression tests tells me that we can leave that
> one untested.
>
I agree
>
> Notes:
>
> This effectively makes the %p prompt (which I use in the example above)
> the same as %:BACKEND_PID: and we may want to note that in the
> documentation.
>
done
> Do we want to change %p to pull from this variable and save the
> snprintf()? Not a measurable savings, more or a don't-repeat-yourself thing.
>
I checked the code, and I don't think so. Current state is safer (I think).
The psql's variables are not protected, and I think, so is safer, better to
read the value for prompt directly by usage of the libpq API instead read
the possibly "customized" variable. I see possible inconsistency,
but again, the same inconsistency can be for variables USER and DBNAME too,
and I am not able to say what is significantly better. Just prompt shows
real value, and the related variable is +/- copy in connection time.
I am not 100% sure in this area what is better, but the change requires
wider and more general discussion, and I don't think the benefits of
possible
change are enough. There are just two possible solutions - we can protect
some psql's variables (and it can do some compatibility issues), or we
need to accept, so the value in prompt can be fake. It is better to not
touch it :-).
>
> In the varlistentry, I suggest we add "This variable is unset when the
> connection is lost." after "but can be changed or unset.
>
done
Regards
Pavel
From 042189d20ca1979b2171412a9c9286ea476d59cf Mon Sep 17 00:00:00 2001
From: "ok...@github.com" <pavel.steh...@gmail.com>
Date: Sat, 4 Feb 2023 18:29:42 +0100
Subject: [PATCH] implementation of BACKEND_PID psql's variable
---
doc/src/sgml/ref/psql-ref.sgml | 16 +++++++++++++++-
src/bin/psql/command.c | 4 ++++
src/bin/psql/help.c | 2 ++
src/test/regress/expected/psql.out | 7 +++++++
src/test/regress/sql/psql.sql | 3 +++
5 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index dc6528dc11..805b0b1d93 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3902,6 +3902,17 @@ bar
</listitem>
</varlistentry>
+ <varlistentry id="app-psql-variables-backend-pid">
+ <term><varname>BACKEND_PID</varname></term>
+ <listitem>
+ <para>
+ The id of server process of the current connection. This is set every
+ time you connect to a database (including program start-up), but can
+ be changed or unset. This variable is unset when the connection is lost.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-psql-variables-comp-keyword-case">
<term><varname>COMP_KEYWORD_CASE</varname></term>
<listitem>
@@ -4548,7 +4559,10 @@ testdb=> <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
<varlistentry id="app-psql-prompting-p">
<term><literal>%p</literal></term>
<listitem>
- <para>The process ID of the backend currently connected to.</para>
+ <para>The process ID of the backend currently connected to.
+ This substitution is almost equal to using <literal>%:BACKEND_PID:</literal>,
+ but it is safer, because psql variable can be overwriten or unset.
+ </para>
</listitem>
</varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b5201edf55..5a9b0e1569 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3783,6 +3783,9 @@ SyncVariables(void)
SetVariable(pset.vars, "PORT", PQport(pset.db));
SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding));
+ snprintf(vbuf, sizeof(vbuf), "%d", PQbackendPID(pset.db));
+ SetVariable(pset.vars, "BACKEND_PID", vbuf);
+
/* 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");
@@ -3817,6 +3820,7 @@ UnsyncVariables(void)
SetVariable(pset.vars, "ENCODING", NULL);
SetVariable(pset.vars, "SERVER_VERSION_NAME", NULL);
SetVariable(pset.vars, "SERVER_VERSION_NUM", NULL);
+ SetVariable(pset.vars, "BACKEND_PID", NULL);
}
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index e45c4aaca5..61c6edd0ba 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -396,6 +396,8 @@ helpVariables(unsigned short int pager)
HELP0(" AUTOCOMMIT\n"
" if set, successful SQL commands are automatically committed\n");
+ HELP0(" BACKEND_PID\n"
+ " id of server process of the current connection\n");
HELP0(" COMP_KEYWORD_CASE\n"
" determines the case used to complete SQL key words\n"
" [lower, upper, preserve-lower, preserve-upper]\n");
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 8fc62cebd2..6613f689a9 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -6578,3 +6578,10 @@ cross-database references are not implemented: "no.such.database"."no.such.schem
cross-database references are not implemented: "no.such.database"."no.such.schema"."no.such.data.type"
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
cross-database references are not implemented: "no.such.database"."no.such.schema"."no.such.extended.statistics"
+-- should be true
+SELECT :BACKEND_PID = pg_backend_pid();
+ ?column?
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 2da9665a19..be4ee1a2da 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1791,3 +1791,6 @@ DROP FUNCTION psql_error;
\dP "no.such.database"."no.such.schema"."no.such.partitioned.relation"
\dT "no.such.database"."no.such.schema"."no.such.data.type"
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
+
+-- should be true
+SELECT :BACKEND_PID = pg_backend_pid();
--
2.39.1
