Horton

Thread

Commits

  1. Add \pset options for boolean value display

  1. psql: add \pset true/false

    Marko Tiikkaja <marko@joh.to> — 2015-10-28T09:03:18Z

    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

  2. Re: psql: add \pset true/false

    Robert Haas <robertmhaas@gmail.com> — 2015-10-28T22:52:15Z

    On Wed, Oct 28, 2015 at 10:03 AM, Marko Tiikkaja <marko@joh.to> 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

  3. Re: psql: add \pset true/false

    Greg Stark <stark@mit.edu> — 2015-10-28T23:38:45Z

    On Wed, Oct 28, 2015 at 10:52 PM, Robert Haas <robertmhaas@gmail.com> 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

  4. Re: psql: add \pset true/false

    Andres Freund <andres@anarazel.de> — 2015-10-28T23:42:56Z

    On 2015-10-28 23:38:45 +0000, Greg Stark wrote:
> On Wed, Oct 28, 2015 at 10:52 PM, Robert Haas <robertmhaas@gmail.com> 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

  5. Re: psql: add \pset true/false

    Marko Tiikkaja <marko@joh.to> — 2015-10-29T00:32:37Z

    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

  6. Re: psql: add \pset true/false

    Robert Haas <robertmhaas@gmail.com> — 2015-10-29T07:50:57Z

    On Thu, Oct 29, 2015 at 1:32 AM, Marko Tiikkaja <marko@joh.to> 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

  7. Re: psql: add \pset true/false

    Daniel Verite <daniel@manitou-mail.org> — 2015-10-29T10:51:04Z

    	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

  8. Re: psql: add \pset true/false

    Marko Tiikkaja <marko@joh.to> — 2015-10-29T11:02:07Z

    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

  9. Re: psql: add \pset true/false

    Matthijs van der Vleuten <matthijs@zr40.nl> — 2015-10-29T12:28:57Z

    On 29 Oct 2015, at 08:50, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> On Thu, Oct 29, 2015 at 1:32 AM, Marko Tiikkaja <marko@joh.to> 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.

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.

In the first screenshot, psql_true_false_off.png, you see the output of psql unpatched. Notice how there’s barely any obvious difference between ’t’ and ‘f’. It’s hard to verify that the supposed output is correct. There’s also some column with all-false values, I would need to carefully examine all the f and t characters to find that.

In the second screenshot, psql_true_false_on.png, I applied Marko’s patch and set a colorful emoji character as the display string for true and false. Now it’s relatively easy to verify that there’s exactly one true value. In addition, it’s hard to miss that the first column is always false (for this particular range). I wasn’t looking for that, but this presentation made it strikingly obvious. In the first screenshot, it would remain hidden. (Did you notice the two columns there that have all false values?)

I’ve been testing Marko’s patch for a few months now, and I’ve found it helps a lot in recognizing booleans in many contexts.

  10. Re: psql: add \pset true/false

    Tom Lane <tgl@sss.pgh.pa.us> — 2015-10-29T13:50:27Z

    Marko Tiikkaja <marko@joh.to> 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

  11. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-11-12T12:50:40Z

    On Thu, Oct 29, 2015 at 10:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Marko Tiikkaja <marko@joh.to> 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

  12. Re: psql: add \pset true/false

    Marko Tiikkaja <marko@joh.to> — 2015-11-12T13:06:28Z

    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

  13. Re: psql: add \pset true/false

    Brendan Jurd <direvus@gmail.com> — 2015-11-12T13:21:01Z

    On Fri, 30 Oct 2015 at 00:51 Tom Lane <tgl@sss.pgh.pa.us> 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

  14. Re: psql: add \pset true/false

    Vik Fearing <vik@2ndquadrant.fr> — 2015-11-12T15:11:27Z

    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

  15. Re: psql: add \pset true/false

    Matthijs van der Vleuten <matthijs@zr40.nl> — 2015-11-12T16:41:49Z

    > On 12 Nov 2015, at 14:21, Brendan Jurd <direvus@gmail.com> wrote:
> 
> On Fri, 30 Oct 2015 at 00:51 Tom Lane <tgl@sss.pgh.pa.us <mailto:tgl@sss.pgh.pa.us>> 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.

  16. Re: psql: add \pset true/false

    Vik Fearing <vik@2ndquadrant.fr> — 2015-11-12T16:51:28Z

    On 11/12/2015 05:41 PM, Matthijs van der Vleuten wrote:
> 
>> On 12 Nov 2015, at 14:21, Brendan Jurd <direvus@gmail.com> wrote:
>>
>> On Fri, 30 Oct 2015 at 00:51 Tom Lane <tgl@sss.pgh.pa.us <mailto:tgl@sss.pgh.pa.us>> 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

  17. Re: psql: add \pset true/false

    Pavel Stehule <pavel.stehule@gmail.com> — 2015-11-12T17:10:46Z

    2015-11-12 17:41 GMT+01:00 Matthijs van der Vleuten <matthijs@zr40.nl>:

>
> On 12 Nov 2015, at 14:21, Brendan Jurd <direvus@gmail.com> wrote:
>
> On Fri, 30 Oct 2015 at 00:51 Tom Lane <tgl@sss.pgh.pa.us> 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

  18. Re: psql: add \pset true/false

    Daniel Verite <daniel@manitou-mail.org> — 2015-11-12T19:38:49Z

    	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

  19. Re: psql: add \pset true/false

    David G. Johnston <david.g.johnston@gmail.com> — 2015-11-12T19:52:12Z

    On Thu, Oct 29, 2015 at 5:28 AM, Matthijs van der Vleuten <matthijs@zr40.nl>
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.​

  20. Re: psql: add \pset true/false

    David G. Johnston <david.g.johnston@gmail.com> — 2015-11-12T20:04:39Z

    On Thu, Oct 29, 2015 at 6:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Marko Tiikkaja <marko@joh.to> 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.

  21. Re: psql: add \pset true/false

    David G. Johnston <david.g.johnston@gmail.com> — 2015-11-12T20:11:49Z

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

> On Thu, Oct 29, 2015 at 6:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Marko Tiikkaja <marko@joh.to> 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.​

  22. Re: psql: add \pset true/false

    Peter Eisentraut <peter_e@gmx.net> — 2015-11-12T20:56:00Z

    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.

  23. Re: psql: add \pset true/false

    Tom Lane <tgl@sss.pgh.pa.us> — 2015-11-12T21:09:40Z

    Peter Eisentraut <peter_e@gmx.net> 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

  24. Re: psql: add \pset true/false

    Jim Nasby <jim.nasby@bluetreble.com> — 2015-11-15T20:20:03Z

    On 11/12/15 3:09 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> 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

  25. Re: psql: add \pset true/false

    Peter Eisentraut <peter_e@gmx.net> — 2015-11-16T01:34:43Z

    On 11/12/15 4:09 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> 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.

  26. Re: psql: add \pset true/false

    Peter Eisentraut <peter_e@gmx.net> — 2015-11-16T01:37:16Z

    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.

  27. Re: psql: add \pset true/false

    Peter Geoghegan <pg@heroku.com> — 2015-11-16T01:43:20Z

    On Thu, Nov 12, 2015 at 1:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter_e@gmx.net> 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

  28. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-02T06:30:28Z

    On Mon, Nov 16, 2015 at 10:43 AM, Peter Geoghegan <pg@heroku.com> wrote:
> On Thu, Nov 12, 2015 at 1:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Peter Eisentraut <peter_e@gmx.net> 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

  29. Re: psql: add \pset true/false

    Daniel Verite <daniel@manitou-mail.org> — 2015-12-02T08:33:39Z

    	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

  30. Re: psql: add \pset true/false

    Jim Nasby <jim.nasby@bluetreble.com> — 2015-12-02T18:10:18Z

    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

  31. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-03T00:24:35Z

    On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby <Jim.Nasby@bluetreble.com> 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

  32. Re: psql: add \pset true/false

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2015-12-03T01:09:23Z

    Hello,

At Thu, 3 Dec 2015 09:24:35 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqTTZXaiVj0_FWvp8hPLLD_yDa8cnS4iuy_HgSOgFz09HA@mail.gmail.com>
> On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby <Jim.Nasby@bluetreble.com> 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),
> +                                    &mustfree);
>          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' ? 'TRRRRUEEE' : "FAAALSE");
     if (*mustfree) free(origcell);
     *mustfree = false;
     break;
   default:
     retcell = origcell;
     break;
   }
   return retcell;
}
=====

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  33. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-03T01:41:11Z

    On Thu, Dec 3, 2015 at 10:09 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello,
>
> At Thu, 3 Dec 2015 09:24:35 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqTTZXaiVj0_FWvp8hPLLD_yDa8cnS4iuy_HgSOgFz09HA@mail.gmail.com>
>> On Thu, Dec 3, 2015 at 3:10 AM, Jim Nasby <Jim.Nasby@bluetreble.com> 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

  34. Re: psql: add \pset true/false

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2015-12-03T05:53:30Z

    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 <michael.paquier@gmail.com> wrote in <CAB7nPqSuLVD0m8GWkL2bar4SuxrwY2+YBYmTBm0UT4At7jg_8Q@mail.gmail.com>
> > 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<pgsql_include> -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    
-----------
 TRRRRUEEE
(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

  35. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-03T06:48:50Z

    On Thu, Dec 3, 2015 at 2:53 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> 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

  36. Re: psql: add \pset true/false

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2015-12-03T07:12:49Z

    Hello, it's disappointing.

At Thu, 3 Dec 2015 15:48:50 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqRLoqdCdGunD3jVuO+ti4beUCL4eu8xLMdjLAXiVcsidA@mail.gmail.com>
> On Thu, Dec 3, 2015 at 2:53 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> 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

  37. Re: psql: add \pset true/false

    Daniel Verite <daniel@manitou-mail.org> — 2015-12-03T10:48:26Z

    	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/YYYY 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

  38. Re: psql: add \pset true/false

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2015-12-04T08:37:54Z

    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

  39. Re: psql: add \pset true/false

    Pavel Stehule <pavel.stehule@gmail.com> — 2015-12-04T09:06:35Z

    2015-12-04 9:37 GMT+01:00 Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp
>:

> 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 +
> +               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 */
> +       if (!fp) return origcell;
> +
> +
> +       /* receive the result from the filter */
> +       if (*mustfree) free(origcell);
> +       getline(&newcell, &celllen, fp);  /* POSIX 2008 */
> +       fclose(fp);
> +       pg_free(cmdline);
> +
> +       /* chop newlines  */
> +       for (p = newcell ; *p && *p != '\n' && *p != '\r' ; p++);
> +       *p = 0;
> +       *mustfree = true;
> +       return newcell;
> +}
> +
>  /*
>   * Use this to print query results
>   *
> @@ -3209,7 +3289,14 @@ printQuery(const PGresult *result, const
> printQueryOpt *opt, FILE *fout, FILE *f
>                                 cell = opt->nullPrint ? opt->nullPrint :
> "";
>                         else
>                         {
> -                               cell = PQgetvalue(result, r, c);
> +                               /* This filter is quite slow */
> +                               if (opt->topt.columnfilter)
> +                                       cell =
> valfilter(opt->topt.columnfilter,
> +
> PQgetvalue(result, r, c),
> +                                                                        c,
> +
> PQftype(result, c),
> +
> &mustfree);
> +
>                                 if (cont.aligns[c] == 'r' &&
> opt->topt.numericLocale)
>                                 {
>                                         cell = format_numeric_locale(cell);
> diff --git a/src/bin/psql/print.h b/src/bin/psql/print.h
> index b0b6bf5..5f2e2b3 100644
> --- a/src/bin/psql/print.h
> +++ b/src/bin/psql/print.h
> @@ -109,6 +109,7 @@ typedef struct printTableOpt
>         unicode_linestyle unicode_border_linestyle;
>         unicode_linestyle unicode_column_linestyle;
>         unicode_linestyle unicode_header_linestyle;
> +       char *columnfilter;     /* the name of column filter program*/
>  } printTableOpt;
>
>  /*
>
> #! /usr/bin/perl
>
> ($type, $colnum, $org) = @ARGV;
>
> if ($type == 16) {  # BOOLOID
>         print ($org eq "t" ? "TRUEEEE" : "FAAALSE");
> } elsif ($type == 25 || $type == 705) {  # TEXTOID || UNKNOWNOID
>         print '$$'.$org.'$$';
> } else {
>         print $type;
> }
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

  40. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-05T12:05:29Z

    On Fri, Dec 4, 2015 at 6:06 PM, Pavel Stehule <pavel.stehule@gmail.com> 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

  41. Re: psql: add \pset true/false

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2015-12-08T10:18:48Z

    Hello,

At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSXcdM-5nFWDf8zuKmW8j_ooE6zYRqYQasp0fjKxKDX2A@mail.gmail.com>
> On Fri, Dec 4, 2015 at 6:06 PM, Pavel Stehule <pavel.stehule@gmail.com> 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

  42. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-08T11:51:56Z

    On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSXcdM-5nFWDf8zuKmW8j_ooE6zYRqYQasp0fjKxKDX2A@mail.gmail.com>
> > 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

  43. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-09T11:50:47Z

    On Tue, Dec 8, 2015 at 8:51 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSXcdM-5nFWDf8zuKmW8j_ooE6zYRqYQasp0fjKxKDX2A@mail.gmail.com>
>> > 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

  44. Re: psql: add \pset true/false

    Michael Paquier <michael.paquier@gmail.com> — 2015-12-10T03:05:05Z

    On Wed, Dec 9, 2015 at 8:50 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Dec 8, 2015 at 8:51 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Tue, Dec 8, 2015 at 7:18 PM, Kyotaro HORIGUCHI
>> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>>> At Sat, 5 Dec 2015 21:05:29 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSXcdM-5nFWDf8zuKmW8j_ooE6zYRqYQasp0fjKxKDX2A@mail.gmail.com>
>>> > 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