Horton

Thread

Commits

  1. In psql's \d commands, don't truncate attribute default values.

  2. Major psql overhaul by Peter Eisentraut.

  1. BUG #16743: psql doesn't show whole expression in stored column

    The Post Office <noreply@postgresql.org> — 2020-11-24T14:17:09Z

    The following bug has been logged on the website:

Bug reference:      16743
Logged by:          David Turon
Email address:      turon.david@seznam.cz
PostgreSQL version: 12.5
Operating system:   CentOS 7
Description:        

Good morning,

when generated column expression length is larger then some value - the rest
of expression is cut in \d[+] output:

CREATE TABLE test(
    a text,
    b text,
    c text,
    d text,    
    ts_vector tsvector GENERATED ALWAYS AS (
            setweight(to_tsvector('simple', COALESCE(a, '')), 'A') || 
            setweight(to_tsvector('simple', COALESCE(b, '')), 'B') ||
            setweight(to_tsvector('simple', COALESCE(c, '')), 'C') ||
            setweight(to_tsvector('simple', COALESCE(d, '')), 'D')) STORED
);

postgres@pgdist:test=# \d test 
                                                                            
               Tabulka "public.test"
  Sloupec  |   Typ    | Collation | Nullable |                              
                                           Implicitně                       
                                                   
-----------+----------+-----------+----------+---------------------------------------------------------------------------------------------------------------------------------------------------------------
 a         | text     |           |          | 
 b         | text     |           |          | 
 c         | text     |           |          | 
 d         | text     |           |          | 
 ts_vector | tsvector |           |          | generated always as
(((setweight(to_tsvector('simple'::regconfig, COALESCE(a, ''::text)),
'A'::"char") || setweight(to_tsvector('simple'::regconfig, ) stored

psql (12.5) server/client

pager is off, only way to show whole expresion is use pg_dump i think. Maybe
it was the intention and isn't bug. Thanks. 

Best regards
David T.

  2. Re: BUG #16743: psql doesn't show whole expression in stored column

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-24T15:46:57Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> when generated column expression length is larger then some value - the rest
> of expression is cut in \d[+] output:

Yeah, this is an intentional and very ancient behavior:

        appendPQExpBufferStr(&buf,
                             ",\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 128)"
                             "\n   FROM pg_catalog.pg_attrdef d"

Maybe we should decide that completeness is more important than keeping
the line to some arbitrary width.  But it's operating as designed.

			regards, tom lane

  3. Re: BUG #16743: psql doesn't show whole expression in stored column

    Bruce Momjian <bruce@momjian.us> — 2020-11-24T15:51:06Z

    On Tue, Nov 24, 2020 at 10:46:57AM -0500, Tom Lane wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
> > when generated column expression length is larger then some value - the rest
> > of expression is cut in \d[+] output:
> 
> Yeah, this is an intentional and very ancient behavior:
> 
>         appendPQExpBufferStr(&buf,
>                              ",\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 128)"
>                              "\n   FROM pg_catalog.pg_attrdef d"
> 
> Maybe we should decide that completeness is more important than keeping
> the line to some arbitrary width.  But it's operating as designed.

I think I am fine with the current behavior.

If you run psql with -E, you can see the queries it generates:

	SELECT a.attname,
	  pg_catalog.format_type(a.atttypid, a.atttypmod),
-->	  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 128)
	          ---------                                                  -------
	   FROM pg_catalog.pg_attrdef d
	   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),
	  a.attnotnull,
	  (SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type t
	   WHERE c.oid = a.attcollation AND t.oid = a.atttypid AND a.attcollation <> t.typcollation) AS attcollation,
	  a.attidentity,
	  a.attgenerated
	FROM pg_catalog.pg_attribute a
	WHERE a.attrelid = '16385' AND a.attnum > 0 AND NOT a.attisdropped
	ORDER BY a.attnum;

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  4. Re: BUG #16743: psql doesn't show whole expression in stored column

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-24T16:25:51Z

    Bruce Momjian <bruce@momjian.us> writes:
> On Tue, Nov 24, 2020 at 10:46:57AM -0500, Tom Lane wrote:
>> Maybe we should decide that completeness is more important than keeping
>> the line to some arbitrary width.  But it's operating as designed.

> I think I am fine with the current behavior.

It's less great in the context of a GENERATED column.  As things stand,
the psql-side code wraps the truncated result into some more syntax:

            else if (generated[0] == ATTRIBUTE_GENERATED_STORED)
                default_str = psprintf("generated always as (%s) stored", PQgetvalue(res, i, attrdef_col));
            else
                /* (note: above we cut off the 'default' string at 128) */
                default_str = PQgetvalue(res, i, attrdef_col);

which seems distinctly more confusing than just truncating the result.
So maybe there's justification here for revisiting that ancient decision.

One could also imagine not appending ") stored" if the attrdef value
appears to have been truncated --- although encoding-conversion effects
would make it hard to tell that for sure.  Perhaps we'd be better off
to move the truncation logic to the psql side?

Another thing here, which is undeniably a bug, is that that psprintf
result is promptly leaked.

			regards, tom lane

  5. Re: BUG #16743: psql doesn't show whole expression in stored column

    David Turoň <turon.david@seznam.cz> — 2020-11-25T07:46:32Z

    Yes ist little bit confusing with truncate only part and leave stored. Maybe
substring to first 125 and append at least '...' or some switch in psql '\
pset cut_row_length on' or something similar.




postgres@pgdist:test=# SELECT a.attname,

  pg_catalog.format_type(a.atttypid, a.atttypmod),

  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 
125)||'...'

   FROM pg_catalog.pg_attrdef d

   WHERE d.adrelid = a.attrelid AND d.adnum = a.attnum AND a.atthasdef),

  a.attnotnull,

  (SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type t

   WHERE c.oid = a.attcollation AND t.oid = a.atttypid AND a.attcollation <>
t.typcollation) AS attcollation,

  a.attidentity,

  a.attgenerated

FROM pg_catalog.pg_attribute a

WHERE a.attrelid = '896567' AND a.attnum > 0 AND NOT a.attisdropped

ORDER BY a.attnum;






-[ RECORD 5 ]+--------------------------------------------------------------
-------------------------------------------------------------------

attname      | ts_vector

format_type  | tsvector

?column?     | ((setweight(to_tsvector('simple'::regconfig, COALESCE(a, ''::
text)), 'A'::"char") || setweight(to_tsvector('simple'::regconfi...

attnotnull   | f

attcollation | <NULL>

attidentity  | 

attgenerated | s





I am using pager, so for me fixed row/column lenght isn't priority.  




regards, David T.



---------- Původní e-mail ----------
Od: Tom Lane <tgl@sss.pgh.pa.us>
Komu: Bruce Momjian <bruce@momjian.us>
Datum: 24. 11. 2020 17:26:07
Předmět: Re: BUG #16743: psql doesn't show whole expression in stored column

"Bruce Momjian <bruce@momjian.us> writes: 
> On Tue, Nov 24, 2020 at 10:46:57AM -0500, Tom Lane wrote: 
>> Maybe we should decide that completeness is more important than keeping 
>> the line to some arbitrary width. But it's operating as designed. 

> I think I am fine with the current behavior. 

It's less great in the context of a GENERATED column. As things stand, 
the psql-side code wraps the truncated result into some more syntax: 

else if (generated[0] == ATTRIBUTE_GENERATED_STORED) 
default_str = psprintf("generated always as (%s) stored", PQgetvalue(res, i,
attrdef_col)); 
else 
/* (note: above we cut off the 'default' string at 128) */ 
default_str = PQgetvalue(res, i, attrdef_col); 

which seems distinctly more confusing than just truncating the result. 
So maybe there's justification here for revisiting that ancient decision. 

One could also imagine not appending ") stored" if the attrdef value 
appears to have been truncated --- although encoding-conversion effects 
would make it hard to tell that for sure. Perhaps we'd be better off 
to move the truncation logic to the psql side? 

Another thing here, which is undeniably a bug, is that that psprintf 
result is promptly leaked. 

regards, tom lane 
"

  6. Re: BUG #16743: psql doesn't show whole expression in stored column

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2020-11-25T08:38:03Z

    On 2020-11-24 16:46, Tom Lane wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
>> when generated column expression length is larger then some value - the rest
>> of expression is cut in \d[+] output:
> 
> Yeah, this is an intentional and very ancient behavior:
> 
>          appendPQExpBufferStr(&buf,
>                               ",\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 128)"
>                               "\n   FROM pg_catalog.pg_attrdef d"
> 
> Maybe we should decide that completeness is more important than keeping
> the line to some arbitrary width.  But it's operating as designed.

I think we should get rid of the truncating.  Otherwise, there is no way 
to actually get the full information, is there?  (Other than pg_dump or 
manual catalog queries.)

  7. Re: BUG #16743: psql doesn't show whole expression in stored column

    Oleksandr Shulgin <oleksandr.shulgin@zalando.de> — 2020-11-25T09:25:29Z

    On Wed, Nov 25, 2020 at 10:19 AM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

> On 2020-11-24 16:46, Tom Lane wrote:
> > PG Bug reporting form <noreply@postgresql.org> writes:
> >> when generated column expression length is larger then some value - the
> rest
> >> of expression is cut in \d[+] output:
> >
> > Yeah, this is an intentional and very ancient behavior:
> >
> >          appendPQExpBufferStr(&buf,
> >                               ",\n  (SELECT
> substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid, true) for 128)"
> >                               "\n   FROM pg_catalog.pg_attrdef d"
> >
> > Maybe we should decide that completeness is more important than keeping
> > the line to some arbitrary width.  But it's operating as designed.
>
> I think we should get rid of the truncating.  Otherwise, there is no way
> to actually get the full information, is there?  (Other than pg_dump or
> manual catalog queries.)
>

Either that or we have to make it clear that the output is truncated:
otherwise it can be mighty confusing.

-- 
Alex

  8. Re: BUG #16743: psql doesn't show whole expression in stored column

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-25T15:03:53Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 2020-11-24 16:46, Tom Lane wrote:
>> Maybe we should decide that completeness is more important than keeping
>> the line to some arbitrary width.  But it's operating as designed.

> I think we should get rid of the truncating.  Otherwise, there is no way 
> to actually get the full information, is there?  (Other than pg_dump or 
> manual catalog queries.)

That'd be okay with me.  It's always seemed a little odd that we do that
for attrdefs but not anything else.

A bit of checking with git blame says that the habit came in with
the very first version of describe.c, in

    commit a45195a191eec367a4f305bb71ab541d17a3b9f9
    Author: Bruce Momjian <bruce@momjian.us>
    Date:   Thu Nov 4 21:56:02 1999 +0000

        Major psql overhaul by Peter Eisentraut.

which has

+    /* Info */
+    cells[i*cols + 2] = xmalloc(128 + 128); /* I'm cutting off the default string at 128 */
+    cells[i*cols + 2][0] = '\0';
+    if (strcmp(PQgetvalue(res, i, 4), "t") == 0)
+        strcat(cells[i*cols + 2], "not null");
+    if (strcmp(PQgetvalue(res, i, 5), "t") == 0) {
+        /* handle "default" here */
+        strcpy(descbuf, "SELECT substring(d.adsrc for 128) FROM pg_attrdef d, pg_class c\n"
+                    "WHERE c.relname = '");

So that looks very much like the truncation was an expedient thing
to do to work with a fixed-size result buffer, rather than something
that was chosen to improve the user experience.

			regards, tom lane

  9. Re: BUG #16743: psql doesn't show whole expression in stored column

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-25T16:15:39Z

    I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> I think we should get rid of the truncating.  Otherwise, there is no way 
>> to actually get the full information, is there?  (Other than pg_dump or 
>> manual catalog queries.)

> That'd be okay with me.  It's always seemed a little odd that we do that
> for attrdefs but not anything else.

Here's a proposed patch for that.

After looking a bit closer, I saw that the memory leak I was worried
about before is not real, because the code passes mustfree = true to
printTableAddCell.  However, I still don't like it one bit, because you
need some undocumented and fragile assumptions about the relationship
between attidentity and attgenerated to conclude that we won't instead
have a false free() attempt on something that mustn't be free'd.  So I
think we should adjust the code to track mustfree explicitly, as done
below.

Should we back-patch this?  I think that the truncation behavior became
significantly more of a problem with the addition of the GENERATED
feature; before that it was clearer what was going on.  So I'm mildly
inclined to back-patch to v12 where that came in.

			regards, tom lane

  10. Re: BUG #16743: psql doesn't show whole expression in stored column

    Bruce Momjian <bruce@momjian.us> — 2020-11-25T16:42:40Z

    On Wed, Nov 25, 2020 at 11:15:39AM -0500, Tom Lane wrote:
> I wrote:
> > Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> >> I think we should get rid of the truncating.  Otherwise, there is no way 
> >> to actually get the full information, is there?  (Other than pg_dump or 
> >> manual catalog queries.)
> 
> > That'd be okay with me.  It's always seemed a little odd that we do that
> > for attrdefs but not anything else.
> 
> Here's a proposed patch for that.
> 
> After looking a bit closer, I saw that the memory leak I was worried
> about before is not real, because the code passes mustfree = true to
> printTableAddCell.  However, I still don't like it one bit, because you
> need some undocumented and fragile assumptions about the relationship
> between attidentity and attgenerated to conclude that we won't instead
> have a false free() attempt on something that mustn't be free'd.  So I
> think we should adjust the code to track mustfree explicitly, as done
> below.
> 
> Should we back-patch this?  I think that the truncation behavior became
> significantly more of a problem with the addition of the GENERATED
> feature; before that it was clearer what was going on.  So I'm mildly
> inclined to back-patch to v12 where that came in.

Works for me.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  11. Re: BUG #16743: psql doesn't show whole expression in stored column

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-11-25T21:20:07Z

    Bruce Momjian <bruce@momjian.us> writes:
> On Wed, Nov 25, 2020 at 11:15:39AM -0500, Tom Lane wrote:
>> Should we back-patch this?  I think that the truncation behavior became
>> significantly more of a problem with the addition of the GENERATED
>> feature; before that it was clearer what was going on.  So I'm mildly
>> inclined to back-patch to v12 where that came in.

> Works for me.

Done that way.

			regards, tom lane