Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Tue, Dec 8, 2015 at 8:51 PM, Michael Paquier
 wrote:
> On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
>  wrote:
>> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier 
>>  wrote in 
>> 
>> > Regarding the patch, I
>> > would tend to think that we should just reject it and try to cruft
>> > something that could be more pluggable if there is really a need.
>> > Thoughts?
>>
>> Honestly saying, I feel similarly with you:p I personally will do
>> something like the following for the original objective.
>
> Are there other opinions? The -1 team is in majority at the end of this 
> thread..

So, marking the patch as rejected? Any objections?
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Wed, Dec 9, 2015 at 8:50 PM, Michael Paquier
 wrote:
> On Tue, Dec 8, 2015 at 8:51 PM, Michael Paquier
>  wrote:
>> On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
>>  wrote:
>>> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier 
>>>  wrote in 
>>> 
>>> > Regarding the patch, I
>>> > would tend to think that we should just reject it and try to cruft
>>> > something that could be more pluggable if there is really a need.
>>> > Thoughts?
>>>
>>> Honestly saying, I feel similarly with you:p I personally will do
>>> something like the following for the original objective.
>>
>> Are there other opinions? The -1 team is in majority at the end of this 
>> thread..
>
> So, marking the patch as rejected? Any objections?

Done so. Alea jacta est, as one guy 2000 years ago would have said.
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Kyotaro HORIGUCHI]

Hello,

At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier  
wrote in 
> On Fri, Dec 4, 2015 at 6:06 PM, Pavel Stehule  wrote:
> > long time I am dream about integrating Lua to psql
> >
> > It is fast enough for these purpose and can be used for custom macros, ..
> 
> Things are perhaps digressing too much here...

But is it another example of per-column pluggable filter? (I
cannot imagine a concrete usage scenario, though.)

> Regarding the patch, I
> would tend to think that we should just reject it and try to cruft
> something that could be more pluggable if there is really a need.
> Thoughts?

Honestly saying, I feel simlilaly with you:p I personally will do
something like the following for the original objective.

psql postgres -c 'select * from tb' | perl -ne 's/t/○/g,s/f/×/g if ($. > 2); 
print'

I think that almost everything of this kind of replacement can be
accomplised by post-filtering like this, especially with
a help of replacement of field separator.


I have shown how it looks that DLL and script/executables is used
for this purpose (the latter could be specified as command line
parameter but totally would be in same shape).  The former looks
too complicated only for this purposr and the latter looks simple
but just peculiar. However, I doubt there's other measure simple
but clean way, and doubt that there's so many usage of pluggable
mechanism on psql.

Quite concise (and unseen) way of platform-independent DLL
loading may allow us to usr DLL for this purpose?

Or, separating platform-independent dll-loading stuff from
backend and share with frontend would allow this? (This could be
acceptable.)


regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
 wrote:
> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier 
>  wrote in 
> 
> > Regarding the patch, I
> > would tend to think that we should just reject it and try to cruft
> > something that could be more pluggable if there is really a need.
> > Thoughts?
>
> Honestly saying, I feel similarly with you:p I personally will do
> something like the following for the original objective.

Are there other opinions? The -1 team is in majority at the end of this thread..
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Fri, Dec 4, 2015 at 6:06 PM, Pavel Stehule  wrote:
> long time I am dream about integrating Lua to psql
>
> It is fast enough for these purpose and can be used for custom macros, ..

Things are perhaps digressing too much here... Regarding the patch, I
would tend to think that we should just reject it and try to cruft
something that could be more pluggable if there is really a need.
Thoughts?
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Pavel Stehule]

2015-12-04 9:37 GMT+01:00 Kyotaro HORIGUCHI :

> Hello, I think this is the my last proposal of an idea on
> psql-side generic solution. Sorry for bothering.
>
> > My environment is CentOS7. But completely forgot Windows
> > environment (or other platforms). I agree with you. It will
> > indeed too much considering all possible platforms.
>
> Ok, DLL is not practical since it would not only be rather
> complex considering multi platform but used only by this feature
> and no other user of DLL is not in sight. It's convincing enough
> for me.
>
> Then, how about calling subprocess for purpose? popen() looks to
> be platform-independent so it is available to call arbitrary
> external programs or scripts.
>
>
> There are several obvious problems on this.
>
>  - Tremendously slow since this executes the program for every value.
>
>  - Dangerous in some aspect. Setting it by environment variable
>is too dengerous but I'm not confidento whether settig by
>\pset is safer as acceptable.
>
> Is it still too complex? Or too slow?
>


long time I am dream about integrating Lua to psql

It is fast enough for these purpose and can be used for custom macros, ..

Regards

Pavel


>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
> index 438a4ec..5dad70b 100644
> --- a/src/bin/psql/command.c
> +++ b/src/bin/psql/command.c
> @@ -1148,7 +1148,8 @@ exec_command(const char *cmd,
>
> int i;
> static const char *const my_list[] = {
> -   "border", "columns", "expanded",
> "fieldsep", "fieldsep_zero",
> +   "border", "columns", "column_filter",
> +   "expanded", "fieldsep", "fieldsep_zero",
> "footer", "format", "linestyle", "null",
> "numericlocale", "pager",
> "pager_min_lines",
> "recordsep", "recordsep_zero",
> @@ -2695,6 +2696,17 @@ do_pset(const char *param, const char *value,
> printQueryOpt *popt, bool quiet)
> if (value)
> popt->topt.columns = atoi(value);
> }
> +   /* set column filter */
> +   else if (strcmp(param, "columnfilter") == 0)
> +   {
> +   if (vallen > 0)
> +   popt->topt.columnfilter = pg_strdup(value);
> +   else
> +   {
> +   if (popt->topt.columnfilter)
> pg_free(popt->topt.columnfilter);
> +   popt->topt.columnfilter = NULL;
> +   }
> +   }
> else
> {
> psql_error("\\pset: unknown option: %s\n", param);
> @@ -2726,6 +2738,15 @@ printPsetInfo(const char *param, struct
> printQueryOpt *popt)
> printf(_("Target width is %d.\n"),
> popt->topt.columns);
> }
>
> +   /* show the target width for the wrapped format */
> +   else if (strcmp(param, "columnfilter") == 0)
> +   {
> +   if (!popt->topt.columnfilter)
> +   printf(_("Column filter is unset.\n"));
> +   else
> +   printf(_("Column filter is \"%s\".\n"),
> popt->topt.columnfilter);
> +   }
> +
> /* show expanded/vertical mode */
> else if (strcmp(param, "x") == 0 || strcmp(param, "expanded") == 0
> || strcmp(param, "vertical") == 0)
> {
> @@ -2938,6 +2959,8 @@ pset_value_string(const char *param, struct
> printQueryOpt *popt)
> return psprintf("%d", popt->topt.border);
> else if (strcmp(param, "columns") == 0)
> return psprintf("%d", popt->topt.columns);
> +   else if (strcmp(param, "columnfilter") == 0)
> +   return pstrdup(popt->topt.columnfilter);
> else if (strcmp(param, "expanded") == 0)
> return pstrdup(popt->topt.expanded == 2
>? "auto"
> diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
> index ad4350e..9731e54 100644
> --- a/src/bin/psql/print.c
> +++ b/src/bin/psql/print.c
> @@ -195,7 +195,7 @@ static void IsPagerNeeded(const printTableContent
> *cont, const int extra_lines,
>   FILE **fout, bool *is_pager);
>
>  static void print_aligned_vertical(const printTableContent *cont, FILE
> *fout);
> -
> +static char *valfilter(char *filtname, char *origcell, int colnum, int
> type, bool *mustfree);
>
>  /* Count number of digits in integral part of number */
>  static int
> @@ -3145,6 +3145,86 @@ printTable(const printTableContent *cont, FILE
> *fout, FILE *flog)
> ClosePager(fout);
>  }
>
> +static char *
> +valfilter(char *filtname, char *origcell, int colnum, int type, bool
> *mustfree)
> +{
> +   char *cmdline;
> +   int buflen = strlen(filtname) + 1 +
> +  

Re: [HACKERS] psql: add \pset true/false

[Kyotaro HORIGUCHI]

Hello, I think this is the my last proposal of an idea on
psql-side generic solution. Sorry for bothering.

> My environment is CentOS7. But completely forgot Windows
> environment (or other platforms). I agree with you. It will
> indeed too much considering all possible platforms.

Ok, DLL is not practical since it would not only be rather
complex considering multi platform but used only by this feature
and no other user of DLL is not in sight. It's convincing enough
for me.

Then, how about calling subprocess for purpose? popen() looks to
be platform-independent so it is available to call arbitrary
external programs or scripts.


There are several obvious problems on this.

 - Tremendously slow since this executes the program for every value.

 - Dangerous in some aspect. Setting it by environment variable
   is too dengerous but I'm not confidento whether settig by
   \pset is safer as acceptable.

Is it still too complex? Or too slow?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 438a4ec..5dad70b 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1148,7 +1148,8 @@ exec_command(const char *cmd,
 
 			int			i;
 			static const char *const my_list[] = {
-"border", "columns", "expanded", "fieldsep", "fieldsep_zero",
+"border", "columns", "column_filter",
+"expanded", "fieldsep", "fieldsep_zero",
 "footer", "format", "linestyle", "null",
 "numericlocale", "pager", "pager_min_lines",
 "recordsep", "recordsep_zero",
@@ -2695,6 +2696,17 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 		if (value)
 			popt->topt.columns = atoi(value);
 	}
+	/* set column filter */
+	else if (strcmp(param, "columnfilter") == 0)
+	{
+		if (vallen > 0)
+			popt->topt.columnfilter = pg_strdup(value);
+		else
+		{
+			if (popt->topt.columnfilter) pg_free(popt->topt.columnfilter);
+			popt->topt.columnfilter = NULL;
+		}
+	}
 	else
 	{
 		psql_error("\\pset: unknown option: %s\n", param);
@@ -2726,6 +2738,15 @@ printPsetInfo(const char *param, struct printQueryOpt *popt)
 			printf(_("Target width is %d.\n"), popt->topt.columns);
 	}
 
+	/* show the target width for the wrapped format */
+	else if (strcmp(param, "columnfilter") == 0)
+	{
+		if (!popt->topt.columnfilter)
+			printf(_("Column filter is unset.\n"));
+		else
+			printf(_("Column filter is \"%s\".\n"), popt->topt.columnfilter);
+	}
+
 	/* show expanded/vertical mode */
 	else if (strcmp(param, "x") == 0 || strcmp(param, "expanded") == 0 || strcmp(param, "vertical") == 0)
 	{
@@ -2938,6 +2959,8 @@ pset_value_string(const char *param, struct printQueryOpt *popt)
 		return psprintf("%d", popt->topt.border);
 	else if (strcmp(param, "columns") == 0)
 		return psprintf("%d", popt->topt.columns);
+	else if (strcmp(param, "columnfilter") == 0)
+		return pstrdup(popt->topt.columnfilter);
 	else if (strcmp(param, "expanded") == 0)
 		return pstrdup(popt->topt.expanded == 2
 	   ? "auto"
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index ad4350e..9731e54 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -195,7 +195,7 @@ static void IsPagerNeeded(const printTableContent *cont, const int extra_lines,
 			  FILE **fout, bool *is_pager);
 
 static void print_aligned_vertical(const printTableContent *cont, FILE *fout);
-
+static char *valfilter(char *filtname, char *origcell, int colnum, int type, bool *mustfree);
 
 /* Count number of digits in integral part of number */
 static int
@@ -3145,6 +3145,86 @@ printTable(const printTableContent *cont, FILE *fout, FILE *flog)
 		ClosePager(fout);
 }
 
+static char *
+valfilter(char *filtname, char *origcell, int colnum, int type, bool *mustfree)
+{
+	char *cmdline;
+	int buflen = strlen(filtname) + 1 +
+		log10(colnum > 0 ? colnum : 1) + 2 + log10(type) + 2 +
+		+ strlen(origcell) + 1 ;
+	FILE *fp;
+	size_t celllen;
+	char *newcell = NULL;
+	char *p;
+	int additional_bytes;
+	bool escape_needed = false;
+
+	/* Check necessity of escaping */
+	for (additional_bytes = 0, p = origcell ; *p ; p++)
+	{
+		if (*p == '\'')
+		{
+			additional_bytes += 4; /* strlen('"'"') - 1 */
+			escape_needed = true;
+		}
+		else if (*p == ' ')
+			escape_needed = true;
+	}
+
+	/* Escaping needed */
+	if (escape_needed)
+	{
+		char *q;
+		char *str;
+		int  newlen;
+
+		additional_bytes += 2;
+		newlen = strlen(origcell) + 1 + additional_bytes;
+		str = (char *)pg_malloc(newlen);
+
+		q = str;
+		*q++ = '\'';
+		for (p = origcell ; *p ; p++)
+		{
+			if (*p == '\'')
+			{
+strcpy(q, "'\"'\"'");
+q += 5;
+			}
+			else
+*q++ = *p;
+		}
+		*q++ = '\'';
+		*q++ = '\0';
+		Assert(q - str == newlen);
+
+		if(*mustfree) free(origcell);
+		origcell = str;
+		*mustfree = true;
+		buflen += additional_bytes;
+	}
+
+	cmdline = pg_malloc(buflen);
+	snprintf(cmdline, buflen, "%s %d %d %s", filtname, type, colnum, origcell);
+	fp = popen(cmdline, "r");
+
+	/* fail silently 

Re: [HACKERS] psql: add \pset true/false

[Daniel Verite]

Jim Nasby wrote:

> I was more thinking it would be nice to be able to temporarily 
> over-ride/wrap what an output function is doing. AFAIK that would allow 
> this to work everywhere (row(), copy, etc). I don't know of any remotely 
> practical way to do that, though.

Yes. Something like:

SET [LOCAL] OUTPUT STYLE FOR  TYPE "typename" TO json_value;

where json_value would represent a set of type-dependant
parameters.
Then the type's output_function and type_modifier_output_function
refered to in CREATE TYPE could use these parameters
to customize the text representation.

For example:
SET output style FOR TYPE bool TO '{"true":"t", "false":"f"}';
or
SET output style FOR TYPE bool TO '{"true":"TRUE", "false":"FALSE"}';

This style declaration doesn't quite fit with GUCs because it should
be bound to a type, but otherwise the behavior is comparable to a
session-wide or transaction-wide SET.

Could be used for date/times too:

SET output style FOR TYPE timestamptz
  TO  '{"format": "DD/MM/ HH24:MI TZ"}';

where applying format would essentially mean
to_char(timestamp, format), which is more flexible
than DateStyle for the output part.


Going even further, maybe some types could support:
SET output style FOR TYPE typename
  TO  '{"function": "funcname"}';

where the function should exist as 
 funcname(typename) returns text
and the type's output_function would just act as a wrapper.

or even:
SET output style FOR TYPE typename
  TO  '{"filter": "funcname"}';
where the function would exist as 
 funcname(text) returns text
and the type's output_function would call funcname
as an output filter for values already in text format.


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


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

Re: [HACKERS] psql: add \pset true/false

[Jim Nasby]

On 11/15/15 7:37 PM, Peter Eisentraut wrote:

On 11/15/15 3:20 PM, Jim Nasby wrote:

As to the argument about displaying a check or an X, why should that
capability only exist for boolean types? For example, why not allow psql
to convert a numeric value into a bar of varying sizes? I've frequently
emulated that with something like SELECT repeat( '*', blah * 30 /
max_of_blah ). I'm sure there's other examples people could think of.


Well, why not?  The question there is only how many marginal features
you want to stuff into psql, not whether it's the right place to stuff them.


I was more thinking it would be nice to be able to temporarily 
over-ride/wrap what an output function is doing. AFAIK that would allow 
this to work everywhere (row(), copy, etc). I don't know of any remotely 
practical way to do that, though.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] psql: add \pset true/false

[Daniel Verite]

Michael Paquier wrote:

> So, looking at this thread, here is the current status:
> - Tom Lane: -1
> - Michael Paquier: -1
> - Peter Geoghegan: +1?
> - Peter Eisentraut: +1
> - the author: surely +1.
> Any other opinions? Feel free to correct this list if needed, and then
> let's try to move on on a consensus if need be to decide the destiny
> of this patch.

In the positive case, the doc should warn somehow about the setting
not being effective when the boolean is inside:
- a field of ROW() type
- an array
- \copy or COPY output
- a field of composite type

(personally this list of exceptions makes me side with the "-1" team).


Other than that,  minor typos to fix (s/prined/printed):

+   fprintf(output, _("  true   set the string to be prined
in place of a TRUE value\n"));
+   fprintf(output, _("  false  set the string to be prined
in place of a FALSE value\n"));



Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby  wrote:
> On 11/15/15 7:37 PM, Peter Eisentraut wrote:
>>
>> On 11/15/15 3:20 PM, Jim Nasby wrote:
>>>
>>> As to the argument about displaying a check or an X, why should that
>>> capability only exist for boolean types? For example, why not allow psql
>>> to convert a numeric value into a bar of varying sizes? I've frequently
>>> emulated that with something like SELECT repeat( '*', blah * 30 /
>>> max_of_blah ). I'm sure there's other examples people could think of.
>>
>>
>> Well, why not?  The question there is only how many marginal features
>> you want to stuff into psql, not whether it's the right place to stuff
>> them.
>
>
> I was more thinking it would be nice to be able to temporarily
> over-ride/wrap what an output function is doing. AFAIK that would allow this
> to work everywhere (row(), copy, etc). I don't know of any remotely
> practical way to do that, though.

You can basically do that with a custom data type and at worse a
custom GUC, no? It does not seem worth bothering the backend with an
extra layer to manage the output of a data type.
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Kyotaro HORIGUCHI]

Hello,

At Thu, 3 Dec 2015 09:24:35 +0900, Michael Paquier  
wrote in 
> On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby  wrote:
> > On 11/15/15 7:37 PM, Peter Eisentraut wrote:
> > I was more thinking it would be nice to be able to temporarily
> > over-ride/wrap what an output function is doing. AFAIK that would allow this
> > to work everywhere (row(), copy, etc). I don't know of any remotely
> > practical way to do that, though.
> 
> You can basically do that with a custom data type and at worse a
> custom GUC, no? It does not seem worth bothering the backend with an
> extra layer to manage the output of a data type.

How about plugins on psql side? Calling hooked function in
printQuery could do that on psql. Impact on psql itself is
minimized. (Of course code for loading is omitted in the below
but it would also small...)

> --- a/src/bin/psql/print.c
> @@ -3210,6 +3210,10 @@ printQuery(const PGresult *result, const printQueryOpt 
> *opt, FILE *fout, FILE *f
>else
>{
>  cell = PQgetvalue(result, r, c);
> +if (outputplugin)
> +  char *cell = outputplugin(cell, PQftype(result, c),
> +);
>  if (cont.aligns[c] == 'r' && opt->topt.numericLocale)
>  {
>cell = format_numeric_locale(cell);



One problem of this is who wants to do this must write a
program. But someone might write general purpose plugin.


=$ loadlib 'outputhook.so';
=$ select 't'::bool;
 bool 
--
 X
(1 row)

=== outputhook.so
char *
outputhook(char *origcell, type, bool *mustfree)
{
   char *retcell;

   switch (type)
   {
   case BOOLOID:
 retcell = (*origcell == 't' ? 'TUEEE' : "FAAALSE");
 if (*mustfree) free(origcell);
 *mustfree = false;
 break;
   default:
 retcell = origcell;
 break;
   }
   return retcell;
}
=

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Thu, Dec 3, 2015 at 2:53 PM, Kyotaro HORIGUCHI
 wrote:
> The attached patch adds a function to load output filter DLL.
> The second file is an example filter module. The following
> commandline with the file can create a test filter module. I
> suppose preload feature only needs additional few lines.

 #include "print.h"
+#include "dlload.h"

I guess that your environment is on Windows... My guess is that you
would need a facility similar to what is in
src/backend/port/dynloader/ but for frontends, and this is not really
worth the move just for this particularly type enforcement in psql.
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Kyotaro HORIGUCHI]

Hello, it's disappointing.

At Thu, 3 Dec 2015 15:48:50 +0900, Michael Paquier  
wrote in 
> On Thu, Dec 3, 2015 at 2:53 PM, Kyotaro HORIGUCHI
>  wrote:
> > The attached patch adds a function to load output filter DLL.
> > The second file is an example filter module. The following
> > commandline with the file can create a test filter module. I
> > suppose preload feature only needs additional few lines.
> 
>  #include "print.h"
> +#include "dlload.h"
> 
> I guess that your environment is on Windows... My guess is that you
> would need a facility similar to what is in
> src/backend/port/dynloader/ but for frontends, and this is not really
> worth the move just for this particularly type enforcement in psql.

My environment is CentOS7. But completely forgot Windows
environment (or other platforms). I agree with you. It will
indeed too much considering all possible platforms.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




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

Re: [HACKERS] psql: add \pset true/false

[Kyotaro HORIGUCHI]

Hello, the attched is an example implement of output filter
dynamic loading feature of psql.

At Thu, 3 Dec 2015 10:41:11 +0900, Michael Paquier  
wrote in 
> > How about plugins on psql side? Calling hooked function in
> > printQuery could do that on psql. Impact on psql itself is
> > minimized. (Of course code for loading is omitted in the below
> > but it would also small...)
> 
> That's interesting, and crazy. You would basically need to have the
> equivalent of \load, or an environment variable like PSQL_SHARED_LIBS
> that is similar to shared_preload_libraries on client-side. In short I
> guess that's actually a clone of LD_PRELOAD.

It is bothersome to share the server-side preload feature so I
made this as a minimal implement. Some safety meature should be
added but not so severe than backend. I home this is an
acceptable mess.

The attached patch adds a function to load output filter DLL.
The second file is an example filter module. The following
commandline with the file can create a test filter module. I
suppose preload feature only needs additional few lines.

gcc -I -fPIC -shared outfunctest.c -o /tmp/outfunctest.so

Then, on psql command line.

=# select 't'::bool;
 bool 
--
 t
=# \load_dll /tmp/outfunctest.so
=# select 't'::bool;
   bool
---
 TUEEE
(1 row)

Anyone can tweak any type of output by this.

Opinions, suggestions or rejections are welcome.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


>From ac5e917a1bd802c37e56dc0861adb8807126d2eb Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Thu, 3 Dec 2015 14:38:03 +0900
Subject: [PATCH] Test implement of dynamic-loadable output filter.

This is a minimum implement so that psql can dynamically load output
filter module. As a test code, this can load only by slash command, no
unload or reload or preloasd feature.
---
 src/bin/psql/Makefile  | 2 +-
 src/bin/psql/command.c | 8 
 src/bin/psql/print.c   | 6 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index f1336d5..2fb1fd1 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -23,7 +23,7 @@ override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/p
 OBJS=	command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
 	startup.o prompt.o variables.o large_obj.o print.o describe.o \
 	tab-complete.o mbprint.o dumputils.o keywords.o kwlookup.o \
-	sql_help.o \
+	sql_help.o dlload.o\
 	$(WIN32RES)
 
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 438a4ec..4101d99 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -40,6 +40,7 @@
 #include "common.h"
 #include "copy.h"
 #include "describe.h"
+#include "dlload.h"
 #include "help.h"
 #include "input.h"
 #include "large_obj.h"
@@ -1001,6 +1002,13 @@ exec_command(const char *cmd,
 		free(opt2);
 	}
 
+	else if (strcmp(cmd, "load_dll") == 0)
+	{
+		char *opt1 = psql_scan_slash_option(scan_state,
+			OT_NORMAL, NULL, true);
+		expand_tilde();
+		success = do_dlload(opt1);
+	}
 
 	/* \o -- set query output */
 	else if (strcmp(cmd, "o") == 0 || strcmp(cmd, "out") == 0)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index ad4350e..37febb2 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -27,6 +27,7 @@
 #include "common.h"
 #include "mbprint.h"
 #include "print.h"
+#include "dlload.h"
 
 /*
  * We define the cancel_pressed flag in this file, rather than common.c where
@@ -3210,6 +3211,11 @@ printQuery(const PGresult *result, const printQueryOpt *opt, FILE *fout, FILE *f
 			else
 			{
 cell = PQgetvalue(result, r, c);
+
+/* Call output filter if registered */
+if (outputfilter)
+	cell = outputfilter(cell, PQftype(result, c), ());
+
 if (cont.aligns[c] == 'r' && opt->topt.numericLocale)
 {
 	cell = format_numeric_locale(cell);
-- 
1.8.3.1

#include 
#include "postgres_fe.h"
#include "catalog/pg_type.h"

char *
outputfilter(char *origcell, int type, bool *mustfree)
{
   char *retcell;

   switch (type)
   {
   case BOOLOID:
 retcell = (*origcell == 't' ? "TUEEE" : "FAAALSE");
 if (*mustfree) free(origcell);
 *mustfree = false;
 break;
   default:
 retcell = origcell;
 break;
   }
   return retcell;
}

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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Thu, Dec 3, 2015 at 10:09 AM, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
> At Thu, 3 Dec 2015 09:24:35 +0900, Michael Paquier 
>  wrote in 
> 
>> On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby  wrote:
>> > On 11/15/15 7:37 PM, Peter Eisentraut wrote:
>> > I was more thinking it would be nice to be able to temporarily
>> > over-ride/wrap what an output function is doing. AFAIK that would allow 
>> > this
>> > to work everywhere (row(), copy, etc). I don't know of any remotely
>> > practical way to do that, though.
>>
>> You can basically do that with a custom data type and at worse a
>> custom GUC, no? It does not seem worth bothering the backend with an
>> extra layer to manage the output of a data type.
>
> How about plugins on psql side? Calling hooked function in
> printQuery could do that on psql. Impact on psql itself is
> minimized. (Of course code for loading is omitted in the below
> but it would also small...)

That's interesting, and crazy. You would basically need to have the
equivalent of \load, or an environment variable like PSQL_SHARED_LIBS
that is similar to shared_preload_libraries on client-side. In short I
guess that's actually a clone of LD_PRELOAD.
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Mon, Nov 16, 2015 at 10:43 AM, Peter Geoghegan  wrote:
> On Thu, Nov 12, 2015 at 1:09 PM, Tom Lane  wrote:
>> Peter Eisentraut  writes:
>>> Plus we already have \pset numericlocale as a similar feature in psql.
>>
>> But \pset numericlocale is also a crock.  It doesn't affect COPY output
>> for instance, and its ability to identify which data types it should apply
>> to is really shaky.  And it's virtually unused, as demonstrated by the
>> fact that serious bugs in it went undetected for many years (cf 4778a0bda,
>> 77130fc14).  That's a really poor advertisement for the usefulness of the
>> proposed feature.
>
> FWIW, I didn't realize that we had "\pset numericlocale" until about a
> year ago. I now use it all the time, and find it very useful.

So, looking at this thread, here is the current status:
- Tom Lane: -1
- Michael Paquier: -1
- Peter Geoghegan: +1?
- Peter Eisentraut: +1
- the author: surely +1.
Any other opinions? Feel free to correct this list if needed, and then
let's try to move on on a consensus if need be to decide the destiny
of this patch.
Regards,
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Jim Nasby]

On 11/12/15 3:09 PM, Tom Lane wrote:

Peter Eisentraut  writes:

Plus we already have \pset numericlocale as a similar feature in psql.


But \pset numericlocale is also a crock.  It doesn't affect COPY output
for instance, and its ability to identify which data types it should apply
to is really shaky.  And it's virtually unused, as demonstrated by the
fact that serious bugs in it went undetected for many years (cf 4778a0bda,
77130fc14).  That's a really poor advertisement for the usefulness of the
proposed feature.


My $0.02 is that something that's type-specific belongs with the type, 
not with clients.


As to the argument about displaying a check or an X, why should that 
capability only exist for boolean types? For example, why not allow psql 
to convert a numeric value into a bar of varying sizes? I've frequently 
emulated that with something like SELECT repeat( '*', blah * 30 / 
max_of_blah ). I'm sure there's other examples people could think of.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] psql: add \pset true/false

[Peter Eisentraut]

On 11/12/15 4:09 PM, Tom Lane wrote:
> Peter Eisentraut  writes:
>> Plus we already have \pset numericlocale as a similar feature in psql.
> 
> But \pset numericlocale is also a crock.  It doesn't affect COPY output
> for instance, and its ability to identify which data types it should apply
> to is really shaky.  And it's virtually unused, as demonstrated by the
> fact that serious bugs in it went undetected for many years (cf 4778a0bda,
> 77130fc14).  That's a really poor advertisement for the usefulness of the
> proposed feature.

Just because people don't find much use for a feature doesn't mean it's
wrong in principle.

Clearly, people occasionally want to change how a data type displays in
psql.  We could implement this in the server.  But then we put a burden
on all non-psql clients to begin all their connections with a growing
number of RESET foo_display calls, and that's not very efficient.  I
think it's up to each client to figure out how to display stuff and to
customize it if necessary.


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

Re: [HACKERS] psql: add \pset true/false

[Peter Eisentraut]

On 11/15/15 3:20 PM, Jim Nasby wrote:
> As to the argument about displaying a check or an X, why should that
> capability only exist for boolean types? For example, why not allow psql
> to convert a numeric value into a bar of varying sizes? I've frequently
> emulated that with something like SELECT repeat( '*', blah * 30 /
> max_of_blah ). I'm sure there's other examples people could think of.

Well, why not?  The question there is only how many marginal features
you want to stuff into psql, not whether it's the right place to stuff them.


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

Re: [HACKERS] psql: add \pset true/false

[Peter Geoghegan]

On Thu, Nov 12, 2015 at 1:09 PM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> Plus we already have \pset numericlocale as a similar feature in psql.
>
> But \pset numericlocale is also a crock.  It doesn't affect COPY output
> for instance, and its ability to identify which data types it should apply
> to is really shaky.  And it's virtually unused, as demonstrated by the
> fact that serious bugs in it went undetected for many years (cf 4778a0bda,
> 77130fc14).  That's a really poor advertisement for the usefulness of the
> proposed feature.

FWIW, I didn't realize that we had "\pset numericlocale" until about a
year ago. I now use it all the time, and find it very useful.

-- 
Peter Geoghegan


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

Re: [HACKERS] psql: add \pset true/false

[David G. Johnston]

On Thu, Nov 12, 2015 at 1:04 PM, David G. Johnston <
david.g.johns...@gmail.com> wrote:

> On Thu, Oct 29, 2015 at 6:50 AM, Tom Lane  wrote:
>
>> Marko Tiikkaja  writes:
>> > On 10/29/15 11:51 AM, Daniel Verite wrote:
>> >> Personally I think it would be worth having, but how about
>> >> booleans inside ROW() or composite types ?
>>
>> > There's not enough information sent over to do that in the client.
>> > Note that this works the same way as  \pset null  with  SELECT
>> > ROW(NULL), so I don't consider it a show stopper for the patch.
>>
>> The problem with that argument is that \pset null is already a kluge
>> (but at least a datatype-independent one).  Now you've added a datatype
>> specific kluge of the same ilk.  It might be useful, it might be short,
>> but that doesn't make it not a kluge.
>>
>> The really key argument that hasn't been addressed here is why does such
>> a behavior belong in psql, rather than elsewhere?  Surely legibility
>> problems aren't unique to psql users.  Moreover, there are exactly
>> parallel facilities for other datatypes on the server side: think
>> DateStyle
>
>
> ​Which provides a finite set of possible values.
> ​
>
>
>> or bytea_output.
>
>
> ​Wasn't this added mostly for performance as opposed to dealing with
> "locale/style" considerations?​
>
> So if you were trying to follow precedent
>> rather than invent a kluge, you'd have submitted a patch to create a GUC
>> that changes the output of boolout().
>>
>>
> ​I'm leaning toward doing this in the client if its offered at all.  An
> unobtrusive usability enhancement - even if limited to non-embedded
> situations - that seems like little effort for a measurable gain.
>
>
​That said whatever is done really wants to be able to interact with at
least "psql \copy" but probably "SQL COPY" as well...again even if just
non-composite outputs.

David J.​

Re: [HACKERS] psql: add \pset true/false

[David G. Johnston]

On Thu, Oct 29, 2015 at 6:50 AM, Tom Lane  wrote:

> Marko Tiikkaja  writes:
> > On 10/29/15 11:51 AM, Daniel Verite wrote:
> >> Personally I think it would be worth having, but how about
> >> booleans inside ROW() or composite types ?
>
> > There's not enough information sent over to do that in the client.
> > Note that this works the same way as  \pset null  with  SELECT
> > ROW(NULL), so I don't consider it a show stopper for the patch.
>
> The problem with that argument is that \pset null is already a kluge
> (but at least a datatype-independent one).  Now you've added a datatype
> specific kluge of the same ilk.  It might be useful, it might be short,
> but that doesn't make it not a kluge.
>
> The really key argument that hasn't been addressed here is why does such
> a behavior belong in psql, rather than elsewhere?  Surely legibility
> problems aren't unique to psql users.  Moreover, there are exactly
> parallel facilities for other datatypes on the server side: think
> DateStyle


​Which provides a finite set of possible values.
​


> or bytea_output.


​Wasn't this added mostly for performance as opposed to dealing with
"locale/style" considerations?​

So if you were trying to follow precedent
> rather than invent a kluge, you'd have submitted a patch to create a GUC
> that changes the output of boolout().
>
>
​I'm leaning toward doing this in the client if its offered at all.  An
unobtrusive usability enhancement - even if limited to non-embedded
situations - that seems like little effort for a measurable gain.

​David J.

Re: [HACKERS] psql: add \pset true/false

[Peter Eisentraut]

On 10/29/15 9:50 AM, Tom Lane wrote:
> The really key argument that hasn't been addressed here is why does such
> a behavior belong in psql, rather than elsewhere?

Because it is the job of the client to mangle the data so that it suits
the purposes of the client.  What comes over the wire is part of the
protocol, not part of the client display.  Language drivers do this sort
of mangling all the time.

> Surely legibility problems aren't unique to psql users.

But the way to deal with it is specific to psql.  In a graphical
environment you might want a checkbox instead, say.  Different clients
will have different requirements and preferences.

> Moreover, there are exactly
> parallel facilities for other datatypes on the server side: think
> DateStyle or bytea_output.

Surely DateStyle is legacy, and bytea_output was actually a performance
feature.  And clients will still mangle either type into their own formats.

Plus we already have \pset numericlocale as a similar feature in psql.



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

Re: [HACKERS] psql: add \pset true/false

[Daniel Verite]

Matthijs van der Vleuten wrote:

> -1 for changing boolout(). It will break anything that receives 
> boolean values from the server.

Not if the default output is still 't' or 'f' like now.
Nobody seems to suggest a gratuitous compatibility break.

>  How a client is going to display values (of any
> type) is logic that should belong in the client, not in the protocol.

Maybe in general, but consider two problems here:

#1: postgres type extensibility implies that clients
display values from types they know nothing about.
It makes sense that they just use the text representation
that comes from the server, unless they have a specific
reason not to.

#2: in the case of built-in types, like boolean, it's not sufficient
to change how the base type gets displayed.
With Marko's patch, array[true,false]  is still displayed as {t,f}
when setting aternatives through the proposed \pset
feature. Same problem inside composite types as mentioned
upthread.

The issue with the patch's approach is that it falls short of
identifying bools in all situations, so the results are inconsistent.
Solving that in psql is hard or possibly irrealistic, because
the reliance on the text representation for complex types goes deep.

By contrast, making the server-side representation configurable seems
easier. Plus it might be of interest for other consumers of resultsets,
outside of psql.

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


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

Re: [HACKERS] psql: add \pset true/false

[Tom Lane]

Peter Eisentraut  writes:
> Plus we already have \pset numericlocale as a similar feature in psql.

But \pset numericlocale is also a crock.  It doesn't affect COPY output
for instance, and its ability to identify which data types it should apply
to is really shaky.  And it's virtually unused, as demonstrated by the
fact that serious bugs in it went undetected for many years (cf 4778a0bda,
77130fc14).  That's a really poor advertisement for the usefulness of the
proposed feature.

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

Re: [HACKERS] psql: add \pset true/false

[David G. Johnston]

On Thu, Oct 29, 2015 at 5:28 AM, Matthijs van der Vleuten 
wrote:

> I have had exactly this situation a week ago. I was testing the output of
> an algorithm that is supposed to have exactly one true value per input id.
>

​If this is particularly important I would add something like (and(col1,
col2, col3, ...) = true) and/or NULLIF((not(col1)::int + not(col2)::int
..., 0) to the grid and check/test those instead of visually scanning the
output.

​If the pretty presentation is destined for final output I'd say you really
want to output text and write a function so that the logic is embedded in
the query and not a side-effect of a specific environment.

David J.​

Re: [HACKERS] psql: add \pset true/false

[Michael Paquier]

On Thu, Oct 29, 2015 at 10:50 PM, Tom Lane  wrote:
> Marko Tiikkaja  writes:
>> On 10/29/15 11:51 AM, Daniel Verite wrote:
>>> Personally I think it would be worth having, but how about
>>> booleans inside ROW() or composite types ?
>
>> There's not enough information sent over to do that in the client.
>> Note that this works the same way as  \pset null  with  SELECT
>> ROW(NULL), so I don't consider it a show stopper for the patch.
>
> The problem with that argument is that \pset null is already a kluge
> (but at least a datatype-independent one).  Now you've added a datatype
> specific kluge of the same ilk.  It might be useful, it might be short,
> but that doesn't make it not a kluge.

FWIW, I am -1 on the concept of enforcing output values for particular
datatypes because that's not the job of psql, and it is easy to do
that at query level (no comment about the existing \pset NULL stuff).

+fprintf(output, _("  true   set the string to be
prined in place of a TRUE value\n"));
+fprintf(output, _("  false  set the string to be
prined in place of a FALSE value\n"));
s/prined/printed/
-- 
Michael


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

Re: [HACKERS] psql: add \pset true/false

[Marko Tiikkaja]

On 11/12/15 1:50 PM, Michael Paquier wrote:

FWIW, I am -1 on the concept of enforcing output values for particular
datatypes because that's not the job of psql


In my view, the job of psql is to make working with a postgres database 
easy for us human beings.  That means (among other things) formatting 
and aligning query output for readability.  I don't see how reformatting 
unreadable boolean values meant for computers is that big of a stretch.



.m


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

Re: [HACKERS] psql: add \pset true/false

[Brendan Jurd]

On Fri, 30 Oct 2015 at 00:51 Tom Lane  wrote:

> The really key argument that hasn't been addressed here is why does such
> a behavior belong in psql, rather than elsewhere?  Surely legibility
> problems aren't unique to psql users.  Moreover, there are exactly
> parallel facilities for other datatypes on the server side: think
> DateStyle or bytea_output.  So if you were trying to follow precedent
> rather than invent a kluge, you'd have submitted a patch to create a GUC
> that changes the output of boolout().
>

I find Tom's analogy to datestyle and bytea_output convincing.

+1 for a GUC that changes the behaviour of boolout.

And also +1 for doing anything at all to improve on the t/f output.  Those
glyphs are way too similar to each other.

I think U+2713 and U+2717 (✓ and ✗) are the obvious choices for a boolean,
but if we have a GUC we can set this to taste.

Cheers,
BJ

Re: [HACKERS] psql: add \pset true/false

[Vik Fearing]

On 10/28/2015 10:03 AM, Marko Tiikkaja wrote:
> Hello hello,
> 
> Since the default t/f output for booleans is not very user friendly,
> attached is a patch which enables you to do for example the following:
> 
> =# \pset true TRUE
> Boolean TRUE display is "TRUE".
> =# \pset false FALSE
> Boolean FALSE display is "FALSE".
> =# select true, false;
>   bool | bool
> --+---
>   TRUE | FALSE
> (1 row)

I think the battle is already lost, but I'm casting my vote in favor of
this patch anyway.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

Re: [HACKERS] psql: add \pset true/false

[Pavel Stehule]

2015-11-12 17:41 GMT+01:00 Matthijs van der Vleuten :

>
> On 12 Nov 2015, at 14:21, Brendan Jurd  wrote:
>
> On Fri, 30 Oct 2015 at 00:51 Tom Lane  wrote:
>
>> The really key argument that hasn't been addressed here is why does such
>> a behavior belong in psql, rather than elsewhere?  Surely legibility
>> problems aren't unique to psql users.  Moreover, there are exactly
>> parallel facilities for other datatypes on the server side: think
>> DateStyle or bytea_output.  So if you were trying to follow precedent
>> rather than invent a kluge, you'd have submitted a patch to create a GUC
>> that changes the output of boolout().
>>
>
> I find Tom's analogy to datestyle and bytea_output convincing.
>
> +1 for a GUC that changes the behaviour of boolout.
>
>
> -1 for changing boolout(). It will break anything that receives boolean
> values from the server. How a client is going to display values (of any
> type) is logic that should belong in the client, not in the protocol.
>

This is not fully valid, because transformation from binary to text is
processed on server side. And client side transformation is easy only for
scalar values.

Regards

Pavel

Re: [HACKERS] psql: add \pset true/false

[Matthijs van der Vleuten]

> On 12 Nov 2015, at 14:21, Brendan Jurd  wrote:
> 
> On Fri, 30 Oct 2015 at 00:51 Tom Lane  > wrote:
> The really key argument that hasn't been addressed here is why does such
> a behavior belong in psql, rather than elsewhere?  Surely legibility
> problems aren't unique to psql users.  Moreover, there are exactly
> parallel facilities for other datatypes on the server side: think
> DateStyle or bytea_output.  So if you were trying to follow precedent
> rather than invent a kluge, you'd have submitted a patch to create a GUC
> that changes the output of boolout().
> 
> I find Tom's analogy to datestyle and bytea_output convincing.
> 
> +1 for a GUC that changes the behaviour of boolout. 

-1 for changing boolout(). It will break anything that receives boolean values 
from the server. How a client is going to display values (of any type) is logic 
that should belong in the client, not in the protocol.

Re: [HACKERS] psql: add \pset true/false

[Vik Fearing]

On 11/12/2015 05:41 PM, Matthijs van der Vleuten wrote:
> 
>> On 12 Nov 2015, at 14:21, Brendan Jurd  wrote:
>>
>> On Fri, 30 Oct 2015 at 00:51 Tom Lane > > wrote:
>> The really key argument that hasn't been addressed here is why does such
>> a behavior belong in psql, rather than elsewhere?  Surely legibility
>> problems aren't unique to psql users.  Moreover, there are exactly
>> parallel facilities for other datatypes on the server side: think
>> DateStyle or bytea_output.  So if you were trying to follow precedent
>> rather than invent a kluge, you'd have submitted a patch to create a GUC
>> that changes the output of boolout().
>>
>> I find Tom's analogy to datestyle and bytea_output convincing.
>>
>> +1 for a GUC that changes the behaviour of boolout. 
> 
> -1 for changing boolout(). It will break anything that receives boolean 
> values from the server. How a client is going to display values (of any type) 
> is logic that should belong in the client, not in the protocol.

I fully agree.  This is something I feel should happen in the client.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

Re: [HACKERS] psql: add \pset true/false

[Tom Lane]

Marko Tiikkaja  writes:
> On 10/29/15 11:51 AM, Daniel Verite wrote:
>> Personally I think it would be worth having, but how about
>> booleans inside ROW() or composite types ?

> There's not enough information sent over to do that in the client.
> Note that this works the same way as  \pset null  with  SELECT 
> ROW(NULL), so I don't consider it a show stopper for the patch.

The problem with that argument is that \pset null is already a kluge
(but at least a datatype-independent one).  Now you've added a datatype
specific kluge of the same ilk.  It might be useful, it might be short,
but that doesn't make it not a kluge.

The really key argument that hasn't been addressed here is why does such
a behavior belong in psql, rather than elsewhere?  Surely legibility
problems aren't unique to psql users.  Moreover, there are exactly
parallel facilities for other datatypes on the server side: think
DateStyle or bytea_output.  So if you were trying to follow precedent
rather than invent a kluge, you'd have submitted a patch to create a GUC
that changes the output of boolout().

Now, that would create a debate about backwards compatibility and whether
making bool output more readable was worth possibly breaking some
applications, but I do not think this patch should escape scrutiny for the
same issue.  There are plenty of people with shell scripts that look at
psql output, which might get broken by careless use of this \pset.

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

Re: [HACKERS] psql: add \pset true/false

[Robert Haas]

On Thu, Oct 29, 2015 at 1:32 AM, Marko Tiikkaja  wrote:
>> 2. If you're the sort of person liable to be confused by t/f, you
>> probably aren't in the target audience for psql anyway.
>
> Really?  The difference between t/f is that the vertical squiggle is
> flipped, THAT'S IT.  Consider:
>
> t t f f f
> f t f t f
>
> Saying that I'm not target audience for not being able to see WTF is going
> on above I find offending.

Sorry, no offense intended.  It's really just never happened to me
that I've had a problem with this, and I've been using psql for quite
a few years now.  I do agree that if you have a bunch of values in a
row it's more apt to be confusing than with just one, but they won't
normally be as closely spaced as you have them there, because psql
inserts spacing and borders and column headers are usually more than
one character.

But I don't really want to argue about this.  I respect your opinion,
and I've given you mine, and wherever we end up based on the opinions
of others is OK with me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] psql: add \pset true/false

[Daniel Verite]

Marko Tiikkaja wrote:

> Since the default t/f output for booleans is not very user friendly, 
> attached is a patch which enables you to do for example the following:

Personally I think it would be worth having, but how about
booleans inside ROW() or composite types ?

test=> \pset true 1
Boolean TRUE display is "1".
test=> \pset false 0
Boolean FALSE display is "0".

test #1: 

test=> select 't'::bool,'f'::bool;
 bool | bool 
--+--
 1| 0

test #2:

test=> select ('t'::bool,'f'::bool);
  row  
---
 (t,f)

test #3:

test=> create type abc as (a bool, b bool, c bool);
test=> select (true,false,true)::abc;
   row   
-
 (t,f,t)


I understand that the patch translates t/f only if the output type
has the OID for bool, which is not the case in #2 or #3, but I guess
users would expect #2 to output (1,0) and #3 (1,0,1).


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


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

Re: [HACKERS] psql: add \pset true/false

[Marko Tiikkaja]

On 10/29/15 11:51 AM, Daniel Verite wrote:

Marko Tiikkaja wrote:


Since the default t/f output for booleans is not very user friendly,
attached is a patch which enables you to do for example the following:


Personally I think it would be worth having, but how about
booleans inside ROW() or composite types ?


There's not enough information sent over to do that in the client.

Note that this works the same way as  \pset null  with  SELECT 
ROW(NULL), so I don't consider it a show stopper for the patch.



.m


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

Re: [HACKERS] psql: add \pset true/false

[Robert Haas]

On Wed, Oct 28, 2015 at 10:03 AM, Marko Tiikkaja  wrote:
> Hello hello,
>
> Since the default t/f output for booleans is not very user friendly,
> attached is a patch which enables you to do for example the following:
>
> =# \pset true TRUE
> Boolean TRUE display is "TRUE".
> =# \pset false FALSE
> Boolean FALSE display is "FALSE".
> =# select true, false;
>   bool | bool
> --+---
>   TRUE | FALSE
> (1 row)
>
>
> (For anyone reviewing: I didn't touch the parts of describe.c which do this
> for nullPrint:
>
>   myopt.nullPrint = NULL;
>
> since I thought it was dubious in the first place, and I don't think we
> output booleans in the describe commands.)

-0 on this concept from me.  I'm not going to vigorously oppose it, but:

1. You can always do it in the query if you really want it.
2. If you're the sort of person liable to be confused by t/f, you
probably aren't in the target audience for psql anyway.
3. I really don't want to end up with a bunch of features of this type
for a variety of different data types.

But I just work here, and if others feel differently, so be it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] psql: add \pset true/false

[Greg Stark]

On Wed, Oct 28, 2015 at 10:52 PM, Robert Haas  wrote:
> 3. I really don't want to end up with a bunch of features of this type
> for a variety of different data types.

We already have \pset null which feels very similar. It's not like 'f'
and 't' are terribly common and probably different from how they
render in whatever driver the user's probably coding to.

On the other hand if their driver isn't mapping booleans to a native
data type and just providing the Postgres text output then then this
is only risking more confusion by presenting the user with a rendering
that's different from what they need to be expecting. IIRC this is the
case for the PHP driver for example.

-- 
greg


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

Re: [HACKERS] psql: add \pset true/false

[Andres Freund]

On 2015-10-28 23:38:45 +, Greg Stark wrote:
> On Wed, Oct 28, 2015 at 10:52 PM, Robert Haas  wrote:
> > 3. I really don't want to end up with a bunch of features of this type
> > for a variety of different data types.
> 
> We already have \pset null which feels very similar.

It's data type neutral, and there's no representation of NULL that's
definitely discernible from the corresponding string. So I don't see
those being the same.


This seems like opening a can of worms to me. Why not add formatting
options in psql for other data types?

- Andres


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

[HACKERS] psql: add \pset true/false

[Marko Tiikkaja]

Hello hello,

Since the default t/f output for booleans is not very user friendly, 
attached is a patch which enables you to do for example the following:


=# \pset true TRUE
Boolean TRUE display is "TRUE".
=# \pset false FALSE
Boolean FALSE display is "FALSE".
=# select true, false;
  bool | bool
--+---
  TRUE | FALSE
(1 row)


(For anyone reviewing: I didn't touch the parts of describe.c which do 
this for nullPrint:


  myopt.nullPrint = NULL;

since I thought it was dubious in the first place, and I don't think we 
output booleans in the describe commands.)




.m
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 212dbfa..2048ba3 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2229,6 +2229,26 @@ lo_import 152801
   
 
   
+  true
+  
+  
+  Sets the string to be printed in place of a boolean TRUE value.
+  The default is to print a 't' character.
+  
+  
+  
+
+  
+  false
+  
+  
+  Sets the string to be printed in place of a boolean FALSE value.
+  The default is to print an 'f' character.
+  
+  
+  
+
+  
   null
   
   
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 5163c76..7fadb0a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1151,7 +1151,7 @@ exec_command(const char *cmd,
 			int			i;
 			static const char *const my_list[] = {
 "border", "columns", "expanded", "fieldsep", "fieldsep_zero",
-"footer", "format", "linestyle", "null",
+"footer", "format", "linestyle", "true", "false", "null",
 "numericlocale", "pager", "pager_min_lines",
 "recordsep", "recordsep_zero",
 "tableattr", "title", "tuples_only",
@@ -2591,6 +2591,26 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet)
 		}
 	}
 
+	/* boolean TRUE display */
+	else if (strcmp(param, "true") == 0)
+	{
+		if (value)
+		{
+			free(popt->truePrint);
+			popt->truePrint = pg_strdup(value);
+		}
+	}
+
+	/* boolean FALSE display */
+	else if (strcmp(param, "false") == 0)
+	{
+		if (value)
+		{
+			free(popt->falsePrint);
+			popt->falsePrint = pg_strdup(value);
+		}
+	}
+
 	/* field separator for unaligned text */
 	else if (strcmp(param, "fieldsep") == 0)
 	{
@@ -2782,6 +2802,20 @@ printPsetInfo(const char *param, struct printQueryOpt *popt)
 			   popt->nullPrint ? popt->nullPrint : "");
 	}
 
+	/* show true display */
+	else if (strcmp(param, "true") == 0)
+	{
+		printf(_("Boolean TRUE display is \"%s\".\n"),
+			   popt->truePrint ? popt->truePrint : "t");
+	}
+
+	/* show false display */
+	else if (strcmp(param, "false") == 0)
+	{
+		printf(_("Boolean FALSE display is \"%s\".\n"),
+			   popt->falsePrint ? popt->falsePrint : "f");
+	}
+
 	/* show locale-aware numeric output */
 	else if (strcmp(param, "numericlocale") == 0)
 	{
@@ -2953,6 +2987,14 @@ pset_value_string(const char *param, struct printQueryOpt *popt)
 		return psprintf("%s", _align2string(popt->topt.format));
 	else if (strcmp(param, "linestyle") == 0)
 		return psprintf("%s", get_line_style(>topt)->name);
+	else if (strcmp(param, "true") == 0)
+		return pset_quoted_string(popt->truePrint
+  ? popt->truePrint
+  : "t");
+	else if (strcmp(param, "false") == 0)
+		return pset_quoted_string(popt->falsePrint
+  ? popt->falsePrint
+  : "f");
 	else if (strcmp(param, "null") == 0)
 		return pset_quoted_string(popt->nullPrint
   ? popt->nullPrint
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 5b63e76..d0bf418 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -258,7 +258,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\H toggle HTML output mode (currently %s)\n"),
 			ON(pset.popt.topt.format == PRINT_HTML));
 	fprintf(output, _("  \\pset [NAME [VALUE]]   set table output option\n"
-	  " (NAME := {format|border|expanded|fieldsep|fieldsep_zero|footer|null|\n"
+	  " (NAME := {format|border|expanded|fieldsep|fieldsep_zero|footer|true|false|null|\n"
 	  " numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager|\n"
 	  " unicode_border_linestyle|unicode_column_linestyle|unicode_header_linestyle})\n"));
 	fprintf(output, _("  \\t [on|off]show only rows (currently %s)\n"),
@@ -371,6 +371,8 @@ helpVariables(unsigned short int pager)
 	fprintf(output, _("  format set output format [unaligned, aligned, wrapped, html, asciidoc, ...]\n"));
 	fprintf(output, _("  footer enable or disable display of the table footer [on, off]\n"));
 	fprintf(output, _("  linestyle  set the border line drawing style [ascii, old-ascii, unicode]\n"));
+	fprintf(output, _("  true   

Re: [HACKERS] psql: add \pset true/false

[Marko Tiikkaja]

On 10/28/15 11:52 PM, Robert Haas wrote:

-0 on this concept from me.  I'm not going to vigorously oppose it, but:

1. You can always do it in the query if you really want it.


True, but not always practical.


2. If you're the sort of person liable to be confused by t/f, you
probably aren't in the target audience for psql anyway.


Really?  The difference between t/f is that the vertical squiggle is 
flipped, THAT'S IT.  Consider:


t t f f f
f t f t f

Saying that I'm not target audience for not being able to see WTF is 
going on above I find offending.



3. I really don't want to end up with a bunch of features of this type
for a variety of different data types.


Fine.  Then let's not add it for a different variety of data types.  But 
boolean is quite essential and it has a really small number of valid 
output values.



.m


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