From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Daniel Verite" <daniel@manitou-mail.org>
cc: "Rahila Syed" <rahilasyed90@gmail.com>,
        "Stephen Frost" <sfrost@snowman.net>,
        "Ashutosh Bapat" <ashutosh.bapat@enterprisedb.com>,
        "pgsql-hackers" <pgsql-hackers@postgresql.org>,
        "Andrew Dunstan" <andrew@dunslane.net>
Subject: Re: Improvements in psql hooks for variables
In-reply-to: <9831.1485896832@sss.pgh.pa.us>
References: <0451dd1e-aad1-4f9c-855e-6cc6584d4b54@manitou-mail.org>
 <30629.1485881533@sss.pgh.pa.us> <9831.1485896832@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 31 Jan 2017 16:07:12 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <31721.1485900676.0@sss.pgh.pa.us>
Date: Tue, 31 Jan 2017 17:13:06 -0500
Message-ID: <31785.1485900786@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <31721.1485900676.1@sss.pgh.pa.us>

BTW ... while I've been fooling with this issue, I've gotten a bit
annoyed at the fact that "\set" prints the variables in, essentially,
creation order.  That makes the list ugly and hard to find things in,
and it exposes some psql implementation details to users.  I propose
the attached simple patch to keep the list in name order instead.

(This is on top of my previous patch, but it'd be pretty trivial
to modify to apply against HEAD.)

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="keep-psql-variable-list-sorted-by-name.patch";
	charset="us-ascii"
Content-ID: <31721.1485900676.2@sss.pgh.pa.us>
Content-Description: keep-psql-variable-list-sorted-by-name.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index b9b8fcb..9ca1000 100644
*** a/src/bin/psql/variables.c
--- b/src/bin/psql/variables.c
*************** valid_variable_name(const char *name)
*** 43,48 ****
--- 43,51 ----
  /*
   * A "variable space" is represented by an otherwise-unused struct _varia=
ble
   * that serves as list header.
+  *
+  * The list entries are kept in name order (according to strcmp).  This
+  * is mainly to make the results of PrintVariables() more pleasing.
   */
  VariableSpace
  CreateVariableSpace(void)
*************** GetVariable(VariableSpace space, const c
*** 74,84 ****
=20=20
  	for (current =3D space->next; current; current =3D current->next)
  	{
! 		if (strcmp(current->name, name) =3D=3D 0)
  		{
  			/* this is correct answer when value is NULL, too */
  			return current->value;
  		}
  	}
=20=20
  	return NULL;
--- 77,91 ----
=20=20
  	for (current =3D space->next; current; current =3D current->next)
  	{
! 		int			cmp =3D strcmp(current->name, name);
!=20
! 		if (cmp =3D=3D 0)
  		{
  			/* this is correct answer when value is NULL, too */
  			return current->value;
  		}
+ 		if (cmp > 0)
+ 			break;				/* it's not there */
  	}
=20=20
  	return NULL;
*************** SetVariable(VariableSpace space, const c
*** 247,253 ****
  		 current;
  		 previous =3D current, current =3D current->next)
  	{
! 		if (strcmp(current->name, name) =3D=3D 0)
  		{
  			/*
  			 * Found entry, so update, unless assign hook returns false.
--- 254,262 ----
  		 current;
  		 previous =3D current, current =3D current->next)
  	{
! 		int			cmp =3D strcmp(current->name, name);
!=20
! 		if (cmp =3D=3D 0)
  		{
  			/*
  			 * Found entry, so update, unless assign hook returns false.
*************** SetVariable(VariableSpace space, const c
*** 293,298 ****
--- 302,309 ----
=20=20
  			return confirmed;
  		}
+ 		if (cmp > 0)
+ 			break;				/* it's not there */
  	}
=20=20
  	/* not present, make new entry ... unless we were asked to delete */
*************** SetVariable(VariableSpace space, const c
*** 303,309 ****
  		current->value =3D pg_strdup(value);
  		current->substitute_hook =3D NULL;
  		current->assign_hook =3D NULL;
! 		current->next =3D NULL;
  		previous->next =3D current;
  	}
  	return true;
--- 314,320 ----
  		current->value =3D pg_strdup(value);
  		current->substitute_hook =3D NULL;
  		current->assign_hook =3D NULL;
! 		current->next =3D previous->next;
  		previous->next =3D current;
  	}
  	return true;
*************** SetVariableHooks(VariableSpace space, co
*** 343,349 ****
  		 current;
  		 previous =3D current, current =3D current->next)
  	{
! 		if (strcmp(current->name, name) =3D=3D 0)
  		{
  			/* found entry, so update */
  			current->substitute_hook =3D shook;
--- 354,362 ----
  		 current;
  		 previous =3D current, current =3D current->next)
  	{
! 		int			cmp =3D strcmp(current->name, name);
!=20
! 		if (cmp =3D=3D 0)
  		{
  			/* found entry, so update */
  			current->substitute_hook =3D shook;
*************** SetVariableHooks(VariableSpace space, co
*** 354,359 ****
--- 367,374 ----
  				(void) (*ahook) (current->value);
  			return;
  		}
+ 		if (cmp > 0)
+ 			break;				/* it's not there */
  	}
=20=20
  	/* not present, make new entry */
*************** SetVariableHooks(VariableSpace space, co
*** 362,368 ****
  	current->value =3D NULL;
  	current->substitute_hook =3D shook;
  	current->assign_hook =3D ahook;
! 	current->next =3D NULL;
  	previous->next =3D current;
  	if (shook)
  		current->value =3D (*shook) (current->value);
--- 377,383 ----
  	current->value =3D NULL;
  	current->substitute_hook =3D shook;
  	current->assign_hook =3D ahook;
! 	current->next =3D previous->next;
  	previous->next =3D current;
  	if (shook)
  		current->value =3D (*shook) (current->value);

------- =_aaaaaaaaaa0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

------- =_aaaaaaaaaa0--