Hello Tom,

So I thought we were done bikeshedding the variable names for this
feature, but as I was reviewing the patch with intent to commit,
I noticed you hadn't updated helpVariables() to mention them.

Indeed.

Possibly you missed this because it doesn't mention VERSION either,
but that doesn't seem very defensible.

Long time ago. Maybe I greped for it to check where it was appearing and did not find what does not exist...

I inserted text to describe all five variables --- but
"SERVER_VERSION_NAME" is too long to fit in the available column space.

Indeed.

In the attached updated patch, I moved all the descriptive text over one
column, and really should have moved it over two columns; but adding even
one space makes a couple of the lines longer than 80 columns when they
were not before.  Since we've blown past 80 columns on some of the other
output, maybe that doesn't matter.  Or maybe we should shorten this
variable name so it doesn't force reformatting of all this text.

It seems that PSQL_EDITOR_LINENUMBER_ARG (25 characters) has been accepted before for an environment variable.

Possible ideas include "DB_VERSION_NAME", "SERVER_VER_NAME", or
"SERVER_VERSION_STR".  (The last saves only one character, whereas
we really need to save two if we're trying not to be wider than any
other documented variable.)

Thoughts?

Like Pavel, I must admit that I do not like these options much, nor the other ones down thread: I hate "hungarian" naming, ISTM that mixing abbrev and full words is better avoided. These are really minor aethetical preferences that I may break occasionally, eg with _NUM for the nice similarity with _NAME.

ISTM that it needs to be consistent with the pre-existing VERSION, which rules out "VER".

Now if this is a bloker, I think that anything is more useful than no variable as it is useful to have them for simple scripting test through server side expressions.

I also like Daniel's idea to update formatting rules, eg following what is done for environment variables and accepting that it won't fit in one page anyway.

  SERVER_VERSION NAME
                bla bla bla

Attached updated patch changes helpVariables() as we'd need to do if
not renaming, and does some minor doc/comment wordsmithing elsewhere.

Given my broken English, I'm fine with wordsmithing.

I like trying to keep the 80 (or 88 it seems) columns limit if possible, because my getting older eyes water on long lines.

In the documentation, I do not think that both SERVER_VERSION_NAME and _NUM (or whatever their chosen name) deserve two independent explanations with heavy repeats just one after the other, and being treated differently from VERSION_*.

The same together-ness approach can be used for helpVariables(), see v8 attached for instance.

Seeing it as is, it calls for having "SERVER_VERSION" as well, but I'm not sure of the better way to get it. I tried with "SELECT VERSION() AS SERVER_VERSION \gset" but varnames are lowerized.

--
Fabien.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index c592eda..79646fd 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3688,6 +3688,19 @@ bar
       </varlistentry>
 
       <varlistentry>
+        <term><varname>SERVER_VERSION_NAME</varname></term>
+        <term><varname>SERVER_VERSION_NUM</varname></term>
+        <listitem>
+        <para>
+        The server's version as a string, for example <literal>9.6.2</>, <literal>10.1</> or <literal>11beta1</>,
+        and in numeric form, for example <literal>90602</>, <literal>100001</>.
+        This is set every time you connect to a database
+        (including program start-up), but can be changed or unset.
+        </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><varname>SINGLELINE</varname></term>
         <listitem>
         <para>
@@ -3733,10 +3746,15 @@ bar
 
       <varlistentry>
         <term><varname>VERSION</varname></term>
+        <term><varname>VERSION_NAME</varname></term>
+        <term><varname>VERSION_NUM</varname></term>
         <listitem>
         <para>
-        This variable is set at program start-up to
-        reflect <application>psql</>'s version.  It can be changed or unset.
+         These variables are set at program start-up to reflect
+         <application>psql</>'s version, respectively as a verbose string,
+         a short string (e.g., <literal>9.6.2</>, <literal>10.1</>,
+         or <literal>11beta1</>), and a number (e.g., <literal>90602</>
+         or <literal>100001</>).  They can be changed or unset.
         </para>
         </listitem>
       </varlistentry>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 96f3a40..237e063 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3337,6 +3337,9 @@ checkWin32Codepage(void)
 void
 SyncVariables(void)
 {
+	char		vbuf[32];
+	const char *server_version;
+
 	/* get stuff from connection */
 	pset.encoding = PQclientEncoding(pset.db);
 	pset.popt.topt.encoding = pset.encoding;
@@ -3348,6 +3351,20 @@ SyncVariables(void)
 	SetVariable(pset.vars, "PORT", PQport(pset.db));
 	SetVariable(pset.vars, "ENCODING", pg_encoding_to_char(pset.encoding));
 
+	/* 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");
+	/* Otherwise fall back on pset.sversion for servers prior 7.4 */
+	if (!server_version)
+	{
+		formatPGVersionNumber(pset.sversion, true, vbuf, sizeof(vbuf));
+		server_version = vbuf;
+	}
+	SetVariable(pset.vars, "SERVER_VERSION_NAME", server_version);
+
+	snprintf(vbuf, sizeof(vbuf), "%d", pset.sversion);
+	SetVariable(pset.vars, "SERVER_VERSION_NUM", vbuf);
+
 	/* send stuff to it, too */
 	PQsetErrorVerbosity(pset.db, pset.verbosity);
 	PQsetErrorContextVisibility(pset.db, pset.show_context);
@@ -3366,6 +3383,8 @@ UnsyncVariables(void)
 	SetVariable(pset.vars, "HOST", NULL);
 	SetVariable(pset.vars, "PORT", NULL);
 	SetVariable(pset.vars, "ENCODING", NULL);
+	SetVariable(pset.vars, "SERVER_VERSION_NAME", NULL);
+	SetVariable(pset.vars, "SERVER_VERSION_NUM", NULL);
 }
 
 
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index b3dbb59..2bef9b3 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -368,11 +368,15 @@ helpVariables(unsigned short int pager)
 	fprintf(output, _("  PROMPT2            specifies the prompt used when a statement continues from a previous line\n"));
 	fprintf(output, _("  PROMPT3            specifies the prompt used during COPY ... FROM STDIN\n"));
 	fprintf(output, _("  QUIET              run quietly (same as -q option)\n"));
+	fprintf(output, _("  SERVER_VERSION_NAME, SERVER_VERSION_NUM\n"));
+	fprintf(output, _("                     server's version (short string, numeric)\n"));
 	fprintf(output, _("  SHOW_CONTEXT       controls display of message context fields [never, errors, always]\n"));
 	fprintf(output, _("  SINGLELINE         end of line terminates SQL command mode (same as -S option)\n"));
 	fprintf(output, _("  SINGLESTEP         single-step mode (same as -s option)\n"));
 	fprintf(output, _("  USER               the currently connected database user\n"));
 	fprintf(output, _("  VERBOSITY          controls verbosity of error reports [default, verbose, terse]\n"));
+	fprintf(output, _("  VERSION, VERSION_NAME, VERSION_NUM\n"));
+	fprintf(output, _("                     psql's version (verbose string, short string, numeric)\n"));
 
 	fprintf(output, _("\nDisplay settings:\n"));
 	fprintf(output, _("Usage:\n"));
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7f76797..1e48f4a 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -160,7 +160,10 @@ main(int argc, char *argv[])
 
 	EstablishVariableSpace();
 
+	/* Create variables showing psql version number */
 	SetVariable(pset.vars, "VERSION", PG_VERSION_STR);
+	SetVariable(pset.vars, "VERSION_NAME", PG_VERSION);
+	SetVariable(pset.vars, "VERSION_NUM", CppAsString2(PG_VERSION_NUM));
 
 	/* Default values for variables (that don't match the result of \unset) */
 	SetVariableBool(pset.vars, "AUTOCOMMIT");
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to