public inbox for [email protected]  
help / color / mirror / Atom feed
From: Isaac Morland <[email protected]>
To: Justin Pryzby <[email protected]>
Cc: Pavel Stehule <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected]
Subject: Re: Remove source code display from \df+?
Date: Sun, 22 Jan 2023 13:59:29 -0500
Message-ID: <CAMsGm5dgogOZ5yNc9BsnFM0n4FuykZREmro9f9gvOBWD_D1-2w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAFj8pRAWVeOm7Vno6bLLVrO+dVZmDNsncvwkc-bPJGh+pifZig@mail.gmail.com>
	<CABUevEwDYJK17rVagSimX_E0SSUXxEGFFi4uCp4xTXEVc41DTQ@mail.gmail.com>
	<CAMsGm5fNUVmeOfw=2MjOG4av+G2iNf5n76yj-4DsG2YCDdOAbQ@mail.gmail.com>
	<CAMsGm5fUOR8eZsHWn+EYDMWaaDbYNcOY7PRULegtL_E9qV=-TA@mail.gmail.com>
	<CAFj8pRCirDcfQb3d7aXYSakG4RMYD4E2D9O2uvJ+M6Z5VDR5Bg@mail.gmail.com>
	<CAMsGm5fWK79PWsEBwG85fHSKCLBhoCEt0ftSq1BuC4_hRaNVgg@mail.gmail.com>
	<[email protected]>
	<CAMsGm5fWWZXiPuixrBVT_jDWWfxnZ2rmZcxagBcRVDPR_KVOGg@mail.gmail.com>
	<CAMsGm5fcJ-XEgDYnOwRdepb-AUJUoToPRzz+RMjZBxuK_Oxonw@mail.gmail.com>
	<[email protected]>

On Sun, 22 Jan 2023 at 00:45, Justin Pryzby <[email protected]> wrote:

> On Sun, Jan 22, 2023 at 12:18:34AM -0500, Isaac Morland wrote:
>


> > It turns out that my tests wanted the owner to be “vagrant” rather than
> > “postgres”. This is apparently because I was running as that user (in a
> > Vagrant VM) when running the tests. Then I took that output and just made
> > it the expected output. I’ve re-worked my build environment a bit so
> that I
> > run as “postgres” inside the Vagrant VM.
> >
> > What I don’t understand is why that didn’t break all the other tests.
>
> Probably because the other tests avoid showing the owner, exactly
> because it varies depending on who runs the tests.  Most of the "plus"
> commands aren't tested, at least in the sql regression tests.
>

Thanks for your patience. I didn’t think about it enough but everything you
both said makes sense.

I’ve re-written the tests to create a test-specific role and functions so
there is no longer a dependency on the superuser name. I pondered the
notion of going with the flow and just leaving out the tests but that
seemed like giving up too easily.

We should probably change one of the CI usernames to something other
> than "postgres" to catch the case that someone hardcodes "postgres".
>
> > proper value in the Owner column so let’s see what CI does with it.
>
> Or better: see above about using it from your github account.


Yes, I will try to get this working before I try to make another
contribution.


Attachments:

  [application/octet-stream] 0001-Remove-source-code-display-from-df-v4.patch (6.3K, ../CAMsGm5dgogOZ5yNc9BsnFM0n4FuykZREmro9f9gvOBWD_D1-2w@mail.gmail.com/3-0001-Remove-source-code-display-from-df-v4.patch)
  download | inline diff:
From d94aaaa00ef9b291440658dd366dafc81b8e7ea2 Mon Sep 17 00:00:00 2001
From: Isaac Morland <[email protected]>
Date: Tue, 17 Jan 2023 14:17:42 -0500
Subject: [PATCH] Remove source code display from \df+

The column is renamed to "Internal name" and will still show the internal
name for C and Internal functions.
---
 doc/src/sgml/ref/psql-ref.sgml     |  3 ++-
 src/bin/psql/describe.c            | 11 +++-----
 src/test/regress/expected/psql.out | 43 ++++++++++++++++++++++++++++++
 src/test/regress/sql/psql.sql      | 35 ++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index dc6528dc11..afdf8668d6 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1650,7 +1650,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
         If the form <literal>\df+</literal> is used, additional information
         about each function is shown, including volatility,
         parallel safety, owner, security classification, access privileges,
-        language, source code and description.
+        language, internal name (for C and Internal language functions only),
+        and description.  Source code can be shown using <literal>\sf</literal>.
         </para>
 
         </listitem>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c8a0bb7b3a..e487fcd480 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -410,14 +410,9 @@ describeFunctions(const char *functypes, const char *func_pattern,
 		appendPQExpBuffer(&buf,
 						  ",\n l.lanname as \"%s\"",
 						  gettext_noop("Language"));
-		if (pset.sversion >= 140000)
-			appendPQExpBuffer(&buf,
-							  ",\n COALESCE(pg_catalog.pg_get_function_sqlbody(p.oid), p.prosrc) as \"%s\"",
-							  gettext_noop("Source code"));
-		else
-			appendPQExpBuffer(&buf,
-							  ",\n p.prosrc as \"%s\"",
-							  gettext_noop("Source code"));
+		appendPQExpBuffer(&buf,
+						  ",\n CASE WHEN l.lanname IN ('internal', 'c') THEN p.prosrc END AS \"%s\"",
+						  gettext_noop("Internal name"));
 		appendPQExpBuffer(&buf,
 						  ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
 						  gettext_noop("Description"));
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 8fc62cebd2..35eb17bd73 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5247,6 +5247,49 @@ reset work_mem;
  pg_catalog | &&   | anyarray      | anyarray       | boolean     | overlaps
 (1 row)
 
+-- check \df+
+CREATE ROLE psql_test_df;
+CREATE OR REPLACE FUNCTION psql_test_df_c (integer, integer, cstring, internal, integer)
+  RETURNS VOID
+  LANGUAGE c
+  IMMUTABLE PARALLEL SAFE STRICT
+AS '$libdir/utf8_and_iso8859', 'utf8_to_iso8859';
+CREATE OR REPLACE FUNCTION psql_test_df_internal (regclass, text)
+  RETURNS bigint
+  LANGUAGE internal
+  PARALLEL SAFE STRICT
+AS 'pg_relation_size';
+CREATE OR REPLACE FUNCTION psql_test_df_sql ()
+  RETURNS VOID
+BEGIN ATOMIC
+  SELECT NULL;
+END;
+CREATE OR REPLACE FUNCTION psql_test_df_plpgsql ()
+  RETURNS VOID
+  LANGUAGE plpgsql
+AS $$
+BEGIN
+  RETURN;
+END;
+$$;
+ALTER FUNCTION psql_test_df_c (integer, integer, cstring, internal, integer)
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_internal (regclass, text)
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_sql ()
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_plpgsql ()
+  OWNER TO psql_test_df;
+\df+ psql_test_df_*
+                                                                                                     List of functions
+ Schema |         Name          | Result data type |             Argument data types              | Type | Volatility | Parallel |    Owner     | Security | Access privileges | Language |  Internal name   | Description 
+--------+-----------------------+------------------+----------------------------------------------+------+------------+----------+--------------+----------+-------------------+----------+------------------+-------------
+ public | psql_test_df_c        | void             | integer, integer, cstring, internal, integer | func | immutable  | safe     | psql_test_df | invoker  |                   | c        | utf8_to_iso8859  | 
+ public | psql_test_df_internal | bigint           | regclass, text                               | func | volatile   | safe     | psql_test_df | invoker  |                   | internal | pg_relation_size | 
+ public | psql_test_df_plpgsql  | void             |                                              | func | volatile   | unsafe   | psql_test_df | invoker  |                   | plpgsql  |                  | 
+ public | psql_test_df_sql      | void             |                                              | func | volatile   | unsafe   | psql_test_df | invoker  |                   | sql      |                  | 
+(4 rows)
+
 -- check \sf
 \sf information_schema._pg_expandarray
 CREATE OR REPLACE FUNCTION information_schema._pg_expandarray(anyarray, OUT x anyelement, OUT n integer)
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 2da9665a19..117f26d92e 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1275,6 +1275,41 @@ reset work_mem;
 \do - pg_catalog.int4
 \do && anyarray *
 
+-- check \df+
+CREATE ROLE psql_test_df;
+CREATE OR REPLACE FUNCTION psql_test_df_c (integer, integer, cstring, internal, integer)
+  RETURNS VOID
+  LANGUAGE c
+  IMMUTABLE PARALLEL SAFE STRICT
+AS '$libdir/utf8_and_iso8859', 'utf8_to_iso8859';
+CREATE OR REPLACE FUNCTION psql_test_df_internal (regclass, text)
+  RETURNS bigint
+  LANGUAGE internal
+  PARALLEL SAFE STRICT
+AS 'pg_relation_size';
+CREATE OR REPLACE FUNCTION psql_test_df_sql ()
+  RETURNS VOID
+BEGIN ATOMIC
+  SELECT NULL;
+END;
+CREATE OR REPLACE FUNCTION psql_test_df_plpgsql ()
+  RETURNS VOID
+  LANGUAGE plpgsql
+AS $$
+BEGIN
+  RETURN;
+END;
+$$;
+ALTER FUNCTION psql_test_df_c (integer, integer, cstring, internal, integer)
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_internal (regclass, text)
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_sql ()
+  OWNER TO psql_test_df;
+ALTER FUNCTION psql_test_df_plpgsql ()
+  OWNER TO psql_test_df;
+\df+ psql_test_df_*
+
 -- check \sf
 \sf information_schema._pg_expandarray
 \sf+ information_schema._pg_expandarray
-- 
2.32.1 (Apple Git-133)



view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Remove source code display from \df+?
  In-Reply-To: <CAMsGm5dgogOZ5yNc9BsnFM0n4FuykZREmro9f9gvOBWD_D1-2w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox