pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Verite <daniel@manitou-mail.org>
Cc: Rahila Syed <rahilasyed90@gmail.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Andrew Dunstan <andrew@dunslane.net>
Subject: Re: Improvements in psql hooks for variables
Date: Tue, 31 Jan 2017 17:13:06 -0500
Message-ID: <31785.1485900786@sss.pgh.pa.us> (raw)
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>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

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

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

Attachments:

keep-psql-variable-list-sorted-by-name.patch (text/x-diff, 3.6K) download 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 _variable * 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 **** for (current = space->next; current; current = current->next) { ! if (strcmp(current->name, name) == 0) { /* this is correct answer when value is NULL, too */ return current->value; } } return NULL; --- 77,91 ---- for (current = space->next; current; current = current->next) { ! int cmp = strcmp(current->name, name); ! ! if (cmp == 0) { /* this is correct answer when value is NULL, too */ return current->value; } + if (cmp > 0) + break; /* it's not there */ } return NULL; *************** SetVariable(VariableSpace space, const c *** 247,253 **** current; previous = current, current = current->next) { ! if (strcmp(current->name, name) == 0) { /* * Found entry, so update, unless assign hook returns false. --- 254,262 ---- current; previous = current, current = current->next) { ! int cmp = strcmp(current->name, name); ! ! if (cmp == 0) { /* * Found entry, so update, unless assign hook returns false. *************** SetVariable(VariableSpace space, const c *** 293,298 **** --- 302,309 ---- return confirmed; } + if (cmp > 0) + break; /* it's not there */ } /* not present, make new entry ... unless we were asked to delete */ *************** SetVariable(VariableSpace space, const c *** 303,309 **** current->value = pg_strdup(value); current->substitute_hook = NULL; current->assign_hook = NULL; ! current->next = NULL; previous->next = current; } return true; --- 314,320 ---- current->value = pg_strdup(value); current->substitute_hook = NULL; current->assign_hook = NULL; ! current->next = previous->next; previous->next = current; } return true; *************** SetVariableHooks(VariableSpace space, co *** 343,349 **** current; previous = current, current = current->next) { ! if (strcmp(current->name, name) == 0) { /* found entry, so update */ current->substitute_hook = shook; --- 354,362 ---- current; previous = current, current = current->next) { ! int cmp = strcmp(current->name, name); ! ! if (cmp == 0) { /* found entry, so update */ current->substitute_hook = shook; *************** SetVariableHooks(VariableSpace space, co *** 354,359 **** --- 367,374 ---- (void) (*ahook) (current->value); return; } + if (cmp > 0) + break; /* it's not there */ } /* not present, make new entry */ *************** SetVariableHooks(VariableSpace space, co *** 362,368 **** current->value = NULL; current->substitute_hook = shook; current->assign_hook = ahook; ! current->next = NULL; previous->next = current; if (shook) current->value = (*shook) (current->value); --- 377,383 ---- current->value = NULL; current->substitute_hook = shook; current->assign_hook = ahook; ! current->next = previous->next; previous->next = current; if (shook) current->value = (*shook) (current->value);

view thread (54+ messages)  latest in thread

Message-ID: <31785.1485900786@sss.pgh.pa.us>
Permalink:  ../31785.1485900786@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/31785.1485900786@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, daniel@manitou-mail.org, rahilasyed90@gmail.com, sfrost@snowman.net, ashutosh.bapat@enterprisedb.com, andrew@dunslane.net
  Subject: Re: Improvements in psql hooks for variables
  In-Reply-To: <31785.1485900786@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox