From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
cc: turon.david@seznam.cz, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16743: psql doesn't show whole expression in stored column
In-reply-to: <426714.1606316633@sss.pgh.pa.us>
References: <16743-7b1bacc4af76e7ad@postgresql.org>
 <286182.1606232817@sss.pgh.pa.us>
 <7a1faa2c-d4d1-67fd-03fe-29ef7e240301@enterprisedb.com>
 <426714.1606316633@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 25 Nov 2020 10:03:53 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <464914.1606320848.0@sss.pgh.pa.us>
Date: Wed, 25 Nov 2020 11:15:39 -0500
Message-ID: <464990.1606320939@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <464914.1606320848.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="undo-attrdef-truncation.patch";
	charset="us-ascii"
Content-ID: <464914.1606320848.2@sss.pgh.pa.us>
Content-Description: undo-attrdef-truncation.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 07d640021c..14150d05a9 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1842,7 +1842,7 @@ describeOneTableDetails(const char *schemaname,
 	{
 		/* use "pretty" mode for expression to avoid excessive parentheses */
 		appendPQExpBufferStr(&buf,
-							 ",\n  (SELECT substring(pg_catalog.pg_get_expr(d.adbin, d.adrelid=
, true) for 128)"
+							 ",\n  (SELECT pg_catalog.pg_get_expr(d.adbin, d.adrelid, true)"
 							 "\n   FROM pg_catalog.pg_attrdef d"
 							 "\n   WHERE d.adrelid =3D a.attrelid AND d.adnum =3D a.attnum AND=
 a.atthasdef)"
 							 ",\n  a.attnotnull");
@@ -2045,7 +2045,8 @@ describeOneTableDetails(const char *schemaname,
 		{
 			char	   *identity;
 			char	   *generated;
-			char	   *default_str =3D "";
+			char	   *default_str;
+			bool		mustfree =3D false;
 =

 			printTableAddCell(&cont, PQgetvalue(res, i, attcoll_col), false, false=
);
 =

@@ -2061,12 +2062,15 @@ describeOneTableDetails(const char *schemaname,
 			else if (identity[0] =3D=3D ATTRIBUTE_IDENTITY_BY_DEFAULT)
 				default_str =3D "generated by default as identity";
 			else if (generated[0] =3D=3D ATTRIBUTE_GENERATED_STORED)
-				default_str =3D psprintf("generated always as (%s) stored", PQgetvalu=
e(res, i, attrdef_col));
+			{
+				default_str =3D psprintf("generated always as (%s) stored",
+									   PQgetvalue(res, i, attrdef_col));
+				mustfree =3D true;
+			}
 			else
-				/* (note: above we cut off the 'default' string at 128) */
 				default_str =3D PQgetvalue(res, i, attrdef_col);
 =

-			printTableAddCell(&cont, default_str, false, generated[0] ? true : fal=
se);
+			printTableAddCell(&cont, default_str, false, mustfree);
 		}
 =

 		/* Info for index columns */

------- =_aaaaaaaaaa0--