Обсуждение: Add \pset options for boolean value display
Вложения
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thu, Mar 20, 2025 at 8:24 PM David G. Johnston <
> david.g.johnston@gmail.com> wrote:
>> It's \pset null for boolean values
> v1, Ready aside from bike-shedding the name.
Do we really want this? It's the sort of thing that has a strong
potential to break anything that reads psql output --- and I'd
urge you to think that human consumers of psql output may well
be the minority. There's an awful lot of scripts out there.
I concede that \pset null hasn't had a huge amount of pushback,
but that doesn't mean that making boolean output unpredictable
will be cost-free. And the costs won't be paid by you (or me),
but by people who didn't ask for it.
regards, tom lane
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thu, Mar 20, 2025 at 8:24 PM David G. Johnston <
> david.g.johnston@gmail.com> wrote:
>> It's \pset null for boolean values
> v1, Ready aside from bike-shedding the name.
Do we really want this? It's the sort of thing that has a strong
potential to break anything that reads psql output --- and I'd
urge you to think that human consumers of psql output may well
be the minority. There's an awful lot of scripts out there.
I concede that \pset null hasn't had a huge amount of pushback,
but that doesn't mean that making boolean output unpredictable
will be cost-free. And the costs won't be paid by you (or me),
but by people who didn't ask for it.
David G. Johnston wrote: > > It's \pset null for boolean values > > > > v1, Ready aside from bike-shedding the name. An annoying weakness of this approach is that it cannot detect booleans inside arrays or composite types or COPY output, meaning that the translation of t/f is incomplete. Also it reminds of a previous discussion (see [1]) where pretty much the same idea was proposed (and eventually rejected at the time). [1] https://postgr.es/m/56308F56.8060908%40joh.to Best regards, -- Daniel Vérité https://postgresql.verite.pro/
David G. Johnston wrote:
> > It's \pset null for boolean values
> >
>
> v1, Ready aside from bike-shedding the name.
An annoying weakness of this approach is that it cannot detect
booleans inside arrays or composite types
or COPY output,
meaning that the translation of t/f is incomplete.
Also it reminds of a previous discussion (see [1]) where pretty much
the same idea was proposed (and eventually rejected at the time).
[1] https://postgr.es/m/56308F56.8060908%40joh.to
On 25/06/2025 00:30, Tom Lane wrote: > "David G. Johnston" <david.g.johnston@gmail.com> writes: >> On Thu, Mar 20, 2025 at 8:24 PM David G. Johnston < >> david.g.johnston@gmail.com> wrote: >>> It's \pset null for boolean values > Do we really want this? Yes, many of us do. > It's the sort of thing that has a strong > potential to break anything that reads psql output --- and I'd > urge you to think that human consumers of psql output may well > be the minority. There's an awful lot of scripts out there. You mean scripts that don't use --no-psqlrc? Those scripts are already bug ridden. -- Vik Fearing
On 2025-Jun-24, David G. Johnston wrote: > v1, Ready aside from bike-shedding the name. Here's v2 after some kibitzing. What do you think? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Вложения
On 2025-Jun-24, David G. Johnston wrote:
> v1, Ready aside from bike-shedding the name.
Here's v2 after some kibitzing. What do you think?
On 2025-Oct-20, David G. Johnston wrote: > Thank you. Seems good from a quick read. I’m regretting the choice of the > display_ prefix; is there any technical limitation or other opposition to > using just true and false? > > \pset true ‘true’ > \pset false ‘false’ > > To keep in line with: > > \pset null ‘(null)’ Uhm. I don't know. No technical limitation AFAICS. It looks a bit weird to me, because those names are so generic; but also I cannot really object to them. That said, such a last-minute bikeshed comment seems like a perfect way to kill your patch. I'll gladly take a vote. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "On the other flipper, one wrong move and we're Fatal Exceptions" (T.U.X.: Term Unit X - http://www.thelinuxreview.com/TUX/)
> On Oct 21, 2025, at 04:51, David G. Johnston <david.g.johnston@gmail.com> wrote: > > On Monday, October 20, 2025, Álvaro Herrera <alvherre@kurilemu.de> wrote: > On 2025-Jun-24, David G. Johnston wrote: > > > v1, Ready aside from bike-shedding the name. > > Here's v2 after some kibitzing. What do you think? > > Thank you. Seems good from a quick read. I’m regretting the choice of the display_ prefix; is there any technical limitationor other opposition to using just true and false? > > \pset true ‘true’ > \pset false ‘false’ > > To keep in line with: > > \pset null ‘(null)’ > +1. Especially, when I see the newly added test case: ``` +prepare q as select null as n, true as t, false as f; +\pset null '(null)' +\pset display_true 'true' +\pset display_false 'false' ``` Looks inconsistant. If we decided to use “display_xx” then we should have renamed “null” to “display_null”. The other thing I am thinking is that, with this patch, users are allowed to display arbitrary strings for true/false, ifa user mistakenly set display_true to f and display_false to t, which will load to misunderstanding. ``` evantest=# \pset display_true f Boolean true display is "f". evantest=# \pset display_false t Boolean false display is "t". evantest=# select true as t, false as f; t | f ---+--- f | t (1 row) ``` Can we perform a basic sanity check to prevent this kind of error-prone behavior? The consideration applies to the “null”option, but since “null” lacks a clear opposite string representation (unlike “true”/“t" and “false”/“f”), it’s fineto skip the check for it. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
The other thing I am thinking is that, with this patch, users are allowed to display arbitrary strings for true/false, if a user mistakenly set display_true to f and display_false to t, which will load to misunderstanding.
On Monday, October 20, 2025, Chao Li <li.evan.chao@gmail.com> wrote:The other thing I am thinking is that, with this patch, users are allowed to display arbitrary strings for true/false, if a user mistakenly set display_true to f and display_false to t, which will load to misunderstanding.Sympathetic to the concern but opposed to taking on such responsibility. They could probably modify their own query to do that if they really wanted to fool someone and I’m having trouble accepting this happening by accident. Do we test for yes/no; oui/non (i.e., foreign language choices); checkmark/X?
> On Oct 21, 2025, at 10:29, David G. Johnston <david.g.johnston@gmail.com> wrote: > > They could probably modify their own query to do that if they really wanted to fool someone and I’m having trouble acceptingthis happening by accident. If they modify queries, the result can visibly correlate to the query, for example: ``` evantest=# select CASE WHEN TRUE THEN 'f' END as t; t --- f (1 row) ``` There is no confusion. But if a user did some test by setting “display_true = f” previous and forget about it, there is ano any indication in current SQL statement but unexpected results might be shown. > Do we test for yes/no; oui/non (i.e., foreign language choices); checkmark/X? > When I said “basic sanity check”, I only meant something like “display_true” cannot be “false” and “f”. I won’t argue more. It’s also reasonable to let users take own responsibilities to stay away from wrong behavior. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Monday, October 20, 2025, David G. Johnston <david.g.johnston@gmail.com>
> wrote:
>> Sympathetic to the concern but opposed to taking on such responsibility.
>> They could probably modify their own query to do that if they really wanted
>> to fool someone and I’m having trouble accepting this happening by
>> accident. Do we test for yes/no; oui/non (i.e., foreign language choices);
>> checkmark/X?
> Actually, preventing t/f makes sense to me. Prevents a “hacker” from
> messing with the default outputs in a hard-to-identify manner. Any other
> value would point to pset being used.
-1. Yeah, you could reject "\pset true 'f'", but what about
not-obviously-different values such as 'f ', or f with a non-breaking
space, or f with a tab, or yadda yadda yadda?
I went on record as opposed to this entire idea back at the start of
this thread, precisely because I was worried that it could lead to
confusion. And I remain of the opinion that it's not a great idea.
But if we're going to do it, let's not bother with any fig-leaf
proposals that pretend to partially guard against confusion.
regards, tom lane
On 2025-Oct-20, David G. Johnston wrote:
> Thank you. Seems good from a quick read. I’m regretting the choice of the
> display_ prefix; is there any technical limitation or other opposition to
> using just true and false?
>
> \pset true ‘true’
> \pset false ‘false’
>
> To keep in line with:
>
> \pset null ‘(null)’
Uhm. I don't know. No technical limitation AFAICS. It looks a bit
weird to me, because those names are so generic; but also I cannot
really object to them. That said, such a last-minute bikeshed comment
seems like a perfect way to kill your patch.
I'll gladly take a vote.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"On the other flipper, one wrong move and we're Fatal Exceptions"
(T.U.X.: Term Unit X - http://www.thelinuxreview.com/TUX/)
On 2025-Oct-21, Álvaro Herrera wrote: > On 2025-Oct-20, David G. Johnston wrote: > > Thank you. Seems good from a quick read. I’m regretting the choice of the > > display_ prefix; is there any technical limitation or other opposition to > > using just true and false? > > > > \pset true ‘true’ > > \pset false ‘false’ > Uhm. I don't know. [...] I'll gladly take a vote. I got zero votes and lots of digression, so I have pushed with your original choice of "display_true" and "display_false". The "true" and "false" variable names sound too generic and I think they're more likely to cause confusion. I think "null" is not a great name either, but it's been there since forever so I'm not going to propose changing it. It's always been the case that for machine-readable output, --no-psqlrc should be used, and a majority of interesting scripts I've seen do that already, so I don't expect lots of breakage. (Also, such a script is easy to fix if anyone runs into trouble.) I have added this to my stock .psqlrc as dogfooding experiment: select :VERSION_NUM >= 190000 as bool_display \gset \if :bool_display \pset display_true YES \pset display_false no \unset bool_display \endif This means I get support for it when connecting to older servers with new psql, and if I use the old psql, things behave normally with no extra noise. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
On 2025-Oct-21, Álvaro Herrera wrote:
> On 2025-Oct-20, David G. Johnston wrote:
> > Thank you. Seems good from a quick read. I’m regretting the choice of the
> > display_ prefix; is there any technical limitation or other opposition to
> > using just true and false?
> >
> > \pset true ‘true’
> > \pset false ‘false’
> Uhm. I don't know. [...] I'll gladly take a vote.
I got zero votes and lots of digression, so I have pushed with your
original choice of "display_true" and "display_false". The "true" and
"false" variable names sound too generic and I think they're more likely
to cause confusion. I think "null" is not a great name either, but it's
been there since forever so I'm not going to propose changing it.
Hi Hackers,
The following script, when built with addresssanitizer, fails with a DoubleFree error. reproduce on master (d3bba0415435)
psql postgres <<EOF
\pset display_false 'f'
SELECT 1 as one, 2 as two \g (display_false=csv csv_fieldsep='\t')
\pset display_false 'f'
EOF
Boolean false display is "f".
one | two
-----+-----
1 | 2
(1 строка) =================================================================
==2263488==ERROR: AddressSanitizer: attempting double-free on 0x774e419e15d0 in thread T0:
#0 0x64c15cb777ab in free.part.0 (/pgpro/builds/master/inst_asan/bin/psql+0x23e7ab) (BuildId: 235e8c12978fc34235af5b00bd4a2feaab9cb794)
#1 0x64c15cbedfbd in do_pset /pgpro/postgres/src/bin/psql/command.c:5310
#2 0x64c15cbef308 in exec_command_pset /pgpro/postgres/src/bin/psql/command.c:2737
#3 0x64c15cbf10d3 in exec_command /pgpro/postgres/src/bin/psql/command.c:431
#4 0x64c15cbf1579 in HandleSlashCmds /pgpro/postgres/src/bin/psql/command.c:258
#5 0x64c15cc25ecd in MainLoop /pgpro/postgres/src/bin/psql/mainloop.c:496
#6 0x64c15cbec791 in process_file /pgpro/postgres/src/bin/psql/command.c:4977
#7 0x64c15cc48cda in main /pgpro/postgres/src/bin/psql/startup.c:424
#8 0x7b2e4322a574 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#9 0x7b2e4322a627 in __libc_start_main_impl ../csu/libc-start.c:360
#10 0x64c15ca91b84 in _start (/pgpro/builds/master/inst_asan/bin/psql+0x158b84) (BuildId: 235e8c12978fc34235af5b00bd4a2feaab9cb794)
0x774e419e15d0 is located 0 bytes inside of 2-byte region [0x774e419e15d0,0x774e419e15d2)
freed by thread T0 here:
#0 0x64c15cb777ab in free.part.0 (/pgpro/builds/master/inst_asan/bin/psql+0x23e7ab) (BuildId: 235e8c12978fc34235af5b00bd4a2feaab9cb794)
#1 0x64c15cbedfbd in do_pset /pgpro/postgres/src/bin/psql/command.c:5310
#2 0x783e419e2980 (<unknown module>)
previously allocated by thread T0 here:
#0 0x64c15cb725c8 in strdup (/pgpro/builds/master/inst_asan/bin/psql+0x2395c8) (BuildId: 235e8c12978fc34235af5b00bd4a2feaab9cb794)
#1 0x64c15cc92bd5 in pg_strdup /pgpro/postgres/src/common/fe_memutils.c:95
SUMMARY: AddressSanitizer: double-free (/pgpro/builds/master/inst_asan/bin/psql+0x23e7ab) (BuildId: 235e8c12978fc34235af5b00bd4a2feaab9cb794) in free.part.0
==2263488==ABORTING
645cb44c5490f70da4dca57b8ecca6562fb883a7 is the first bad commit
gcc --version
gcc (Ubuntu 15.2.0-4ubuntu4) 15.2.0
building CPPFLAGS="-Og -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fno-sanitize=nonnull-attribute -fstack-protector" LDFLAGS='-fsanitize=address -fsanitize=undefined -static-libasan' \
./configure --prefix /pgpro/builds/master_simple && make world-bin -s -j$(nproc) && make install-world-bin -s
On Monday, November 3, 2025, Álvaro Herrera <alvherre@kurilemu.de> wrote:On 2025-Oct-21, Álvaro Herrera wrote:
> On 2025-Oct-20, David G. Johnston wrote:
> > Thank you. Seems good from a quick read. I’m regretting the choice of the
> > display_ prefix; is there any technical limitation or other opposition to
> > using just true and false?
> >
> > \pset true ‘true’
> > \pset false ‘false’
> Uhm. I don't know. [...] I'll gladly take a vote.
I got zero votes and lots of digression, so I have pushed with your
original choice of "display_true" and "display_false". The "true" and
"false" variable names sound too generic and I think they're more likely
to cause confusion. I think "null" is not a great name either, but it's
been there since forever so I'm not going to propose changing it.Thank you.David J.
The following script, when built with addresssanitizer, fails with a DoubleFree error. reproduce on master (d3bba0415435)
On Monday, April 20, 2026, a.kozhemyakin <a.kozhemyakin@postgrespro.ru> wrote:The following script, when built with addresssanitizer, fails with a DoubleFree error. reproduce on master (d3bba0415435)
Thanks for the report. To be honest, I’m not sure where to start debugging it - but I plan to give it a go. I’m curious if it fails for “\pset null ‘N’” “\g (null=csv)” too and I just got bit by inheriting its layout. Don’t see why these new ones would be different.