Remove source code display from \df+?

st 11. 1. 2023 v 17:50 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

I find \df+ much less useful than it should be because it tends to be
cluttered up with source code. Now that we have \sf, would it be reasonable
to remove the source code from the \df+ display? This would make it easier
to see function permissions and comments. If somebody wants to see the full
definition of a function they can always invoke \sf on it.

If there is consensus on the idea in principle I will write up a patch.

+1

Pavel

On Wed, Jan 11, 2023 at 6:19 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

st 11. 1. 2023 v 17:50 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

I find \df+ much less useful than it should be because it tends to be
cluttered up with source code. Now that we have \sf, would it be reasonable
to remove the source code from the \df+ display? This would make it easier
to see function permissions and comments. If somebody wants to see the full
definition of a function they can always invoke \sf on it.

If there is consensus on the idea in principle I will write up a patch.

+1

+1 but maybe with a twist. For any functions in a procedural language like
plpgsql, it makes it pretty useless today. But when viewing an internal or
C language function, it's short enough and still actually useful. Maybe
some combination where it would keep showing those for such language, but
would show "use \sf to view source" for procedural languages?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

st 11. 1. 2023 v 18:25 odesílatel Magnus Hagander <magnus@hagander.net>
napsal:

On Wed, Jan 11, 2023 at 6:19 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

st 11. 1. 2023 v 17:50 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

I find \df+ much less useful than it should be because it tends to be
cluttered up with source code. Now that we have \sf, would it be reasonable
to remove the source code from the \df+ display? This would make it easier
to see function permissions and comments. If somebody wants to see the full
definition of a function they can always invoke \sf on it.

If there is consensus on the idea in principle I will write up a patch.

+1

+1 but maybe with a twist. For any functions in a procedural language like
plpgsql, it makes it pretty useless today. But when viewing an internal or
C language function, it's short enough and still actually useful. Maybe
some combination where it would keep showing those for such language, but
would show "use \sf to view source" for procedural languages?

yes, it is almost necessary for C functions or functions in external
languages. Maybe it can be specified in pg_language if prosrc is really
source code or some reference.

Right, for internal or C functions it just gives a symbol name or something
similar. I've never been annoyed seeing that, just having pages of PL/PGSQL
(I use a lot of that, possibly biased towards the “too much” direction)
take up all the space.

A bit hacky, but what about only showing the first line of the source code?
Then you would see link names for that type of function but the main
benefit of suppressing the full source code would be obtained. Or, show
source if it is a single line, otherwise “…” (as in, literally an ellipsis).

On Wed, 11 Jan 2023 at 12:31, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

st 11. 1. 2023 v 18:57 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

Right, for internal or C functions it just gives a symbol name or
something similar. I've never been annoyed seeing that, just having pages
of PL/PGSQL (I use a lot of that, possibly biased towards the “too much”
direction) take up all the space.

A bit hacky, but what about only showing the first line of the source
code? Then you would see link names for that type of function but the main
benefit of suppressing the full source code would be obtained. Or, show
source if it is a single line, otherwise “…” (as in, literally an ellipsis).

please, don't send top post replies -
https://en.wikipedia.org/wiki/Posting_style

I don't think printing a few first rows is a good idea - usually there is
nothing interesting (same is PL/Perl, PL/Python, ...)

If the proposed feature can be generic, then this information should be
stored somewhere in pg_language. Or we can redesign usage of prosrc and
probin columns - but this can be a much more massive change.

Regards

Pavel

Or, only show the source in \df++. But it'd be a bit unfortunate if the
C language function wasn't shown in \df+

On Wed, 11 Jan 2023 at 13:11, Pavel Stehule <pavel.stehule@gmail.com> wrote:

please, don't send top post replies -

https://en.wikipedia.org/wiki/Posting_style

Sorry about that; I do know to do it properly and usually get it right.
GMail doesn’t seem to have an option (that I can find) to leave no space at
the top and put my cursor at the bottom; it nudges pretty firmly in the
direction of top-posting. Thanks for the reminder.

I don't think printing a few first rows is a good idea - usually there is
nothing interesting (same is PL/Perl, PL/Python, ...)

If the proposed feature can be generic, then this information should be
stored somewhere in pg_language. Or we can redesign usage of prosrc and
probin columns - but this can be a much more massive change.

<http://www.redpill-linpro.com/&gt;

I’m looking for a quick win. So I think that means either drop the source
column entirely, or show single-line source values only and nothing or a
placeholder for anything that is more than one line, unless somebody comes
up with another suggestion. Originally I was thinking just to remove
entirely, but I’ve seen a couple of comments suggesting that people would
find it unfortunate if the source weren’t shown for internal/C functions,
so now I’m leaning towards showing single-line values only.

I agree that showing the first line or couple of lines isn't likely to be
very useful. The way I format my functions, the first line is always blank
anyway: I write the bodies like this:

$$
BEGIN
…
END;
$$;

Even if somebody uses a different style, the first line is probably just
"BEGIN" or something equally formulaic.

On Wed, Jan 11, 2023 at 7:24 PM Isaac Morland <isaac.morland@gmail.com>
wrote:

On Wed, 11 Jan 2023 at 13:11, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

please, don't send top post replies -

https://en.wikipedia.org/wiki/Posting_style

Sorry about that; I do know to do it properly and usually get it right.
GMail doesn’t seem to have an option (that I can find) to leave no space at
the top and put my cursor at the bottom; it nudges pretty firmly in the
direction of top-posting. Thanks for the reminder.

I don't think printing a few first rows is a good idea - usually there is
nothing interesting (same is PL/Perl, PL/Python, ...)

If the proposed feature can be generic, then this information should be
stored somewhere in pg_language. Or we can redesign usage of prosrc and
probin columns - but this can be a much more massive change.

<http://www.redpill-linpro.com/&gt;

I’m looking for a quick win. So I think that means either drop the source
column entirely, or show single-line source values only and nothing or a
placeholder for anything that is more than one line, unless somebody comes
up with another suggestion. Originally I was thinking just to remove
entirely, but I’ve seen a couple of comments suggesting that people would
find it unfortunate if the source weren’t shown for internal/C functions,
so now I’m leaning towards showing single-line values only.

I agree that showing the first line or couple of lines isn't likely to be
very useful. The way I format my functions, the first line is always blank
anyway: I write the bodies like this:

$$
BEGIN
…
END;
$$;

Even if somebody uses a different style, the first line is probably just
"BEGIN" or something equally formulaic.

This is only about Internal and C, isn't it? Isn't the oid of these static,
and identified by INTERNALlanguageId and ClanguageId respectively? So we
could just have the query show the prosrc column if the language oid is one
of those two, and otherwise show "Please use \sf to view the source"?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

st 11. 1. 2023 v 19:31 odesílatel Magnus Hagander <magnus@hagander.net>
napsal:

On Wed, Jan 11, 2023 at 7:24 PM Isaac Morland <isaac.morland@gmail.com>
wrote:

On Wed, 11 Jan 2023 at 13:11, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

please, don't send top post replies -

https://en.wikipedia.org/wiki/Posting_style

Sorry about that; I do know to do it properly and usually get it right.
GMail doesn’t seem to have an option (that I can find) to leave no space at
the top and put my cursor at the bottom; it nudges pretty firmly in the
direction of top-posting. Thanks for the reminder.

I don't think printing a few first rows is a good idea - usually there
is nothing interesting (same is PL/Perl, PL/Python, ...)

If the proposed feature can be generic, then this information should be
stored somewhere in pg_language. Or we can redesign usage of prosrc and
probin columns - but this can be a much more massive change.

<http://www.redpill-linpro.com/&gt;

I’m looking for a quick win. So I think that means either drop the source
column entirely, or show single-line source values only and nothing or a
placeholder for anything that is more than one line, unless somebody comes
up with another suggestion. Originally I was thinking just to remove
entirely, but I’ve seen a couple of comments suggesting that people would
find it unfortunate if the source weren’t shown for internal/C functions,
so now I’m leaning towards showing single-line values only.

I agree that showing the first line or couple of lines isn't likely to be
very useful. The way I format my functions, the first line is always blank
anyway: I write the bodies like this:

$$
BEGIN
…
END;
$$;

Even if somebody uses a different style, the first line is probably just
"BEGIN" or something equally formulaic.

This is only about Internal and C, isn't it? Isn't the oid of these
static, and identified by INTERNALlanguageId and ClanguageId respectively?
So we could just have the query show the prosrc column if the language oid
is one of those two, and otherwise show "Please use \sf to view the
source"?

I think it can be acceptable - maybe we can rename the column "source code"
like "internal name" or some like that.

again I don't think printing "Please use \sf to view the source"? " often
can be user friendly. \? is clear and \sf is easy to use

Pavel Stehule <pavel.stehule@gmail.com> writes:

st 11. 1. 2023 v 19:31 odesílatel Magnus Hagander <magnus@hagander.net>
napsal:

This is only about Internal and C, isn't it? Isn't the oid of these
static, and identified by INTERNALlanguageId and ClanguageId respectively?
So we could just have the query show the prosrc column if the language oid
is one of those two, and otherwise show "Please use \sf to view the
source"?

I think it can be acceptable - maybe we can rename the column "source code"
like "internal name" or some like that.

Yeah, "source code" has always been kind of a misnomer for these
languages.

again I don't think printing "Please use \sf to view the source"? " often
can be user friendly. \? is clear and \sf is easy to use

We could shorten it to "See \sf" or something like that. But if we change
the column header to "internal name" or the like, then the column just
obviously doesn't apply for non-internal languages, so leaving it null
should be fine.

regards, tom lane

st 11. 1. 2023 v 22:11 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

st 11. 1. 2023 v 19:31 odesílatel Magnus Hagander <magnus@hagander.net>
napsal:

This is only about Internal and C, isn't it? Isn't the oid of these
static, and identified by INTERNALlanguageId and ClanguageId

respectively?

So we could just have the query show the prosrc column if the language

oid

is one of those two, and otherwise show "Please use \sf to view the
source"?

I think it can be acceptable - maybe we can rename the column "source

code"

like "internal name" or some like that.

Yeah, "source code" has always been kind of a misnomer for these
languages.

again I don't think printing "Please use \sf to view the source"? "

often

can be user friendly. \? is clear and \sf is easy to use

We could shorten it to "See \sf" or something like that. But if we change
the column header to "internal name" or the like, then the column just
obviously doesn't apply for non-internal languages, so leaving it null
should be fine.

+1

On Thu, Jan 12, 2023 at 6:23 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

st 11. 1. 2023 v 22:11 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Pavel Stehule <pavel.stehule@gmail.com> writes:

st 11. 1. 2023 v 19:31 odesílatel Magnus Hagander <magnus@hagander.net>
napsal:

This is only about Internal and C, isn't it? Isn't the oid of these
static, and identified by INTERNALlanguageId and ClanguageId

respectively?

So we could just have the query show the prosrc column if the language

oid

is one of those two, and otherwise show "Please use \sf to view the
source"?

I think it can be acceptable - maybe we can rename the column "source

code"

like "internal name" or some like that.

Yeah, "source code" has always been kind of a misnomer for these
languages.

again I don't think printing "Please use \sf to view the source"? "

often

can be user friendly. \? is clear and \sf is easy to use

We could shorten it to "See \sf" or something like that. But if we change
the column header to "internal name" or the like, then the column just
obviously doesn't apply for non-internal languages, so leaving it null
should be fine.

+1

Sure, that works for me as well. I agree the suggested text was way too
long, I was more thinking of "something in this direction" rather than that
exact text. But yes, with a change of names, we can leave it NULL as well.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On Thu, 12 Jan 2023 at 10:04, Magnus Hagander <magnus@hagander.net> wrote:

We could shorten it to "See \sf" or something like that. But if we change

the column header to "internal name" or the like, then the column just
obviously doesn't apply for non-internal languages, so leaving it null
should be fine.

+1

Sure, that works for me as well. I agree the suggested text was way too
long, I was more thinking of "something in this direction" rather than that
exact text. But yes, with a change of names, we can leave it NULL as well.

Thanks everybody. So based on the latest discussion I will:

1) rename the column from “Source code” to “Internal name”; and
2) change the contents to NULL except when the language (identified by oid)
is INTERNAL or C.

Patch forthcoming, I hope.

On Thu, 12 Jan 2023 at 12:06, Isaac Morland <isaac.morland@gmail.com> wrote:

Thanks everybody. So based on the latest discussion I will:

1) rename the column from “Source code” to “Internal name”; and
2) change the contents to NULL except when the language (identified by
oid) is INTERNAL or C.

Patch forthcoming, I hope.

I've attached a patch for this. It turns out to simplify the existing code
in one way because the recently added call to pg_get_function_sqlbody() is
no longer needed since it applies only to SQL functions, which will now
display as a blank column.

I implemented the change and was surprised to see that no tests failed.
Turns out that while there are several tests for \df, there were none for
\df+. I added a couple, just using \df+ on some functions that appear to me
to be present on plain vanilla Postgres.

I was initially concerned about translation support for the column heading,
but it turns out that \dT+ already has a column with the exact same name so
it appears we don’t need any new translations.

I welcome comments and feedback. Now to try to find something manageable to
review.

Hi

út 17. 1. 2023 v 20:29 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

On Thu, 12 Jan 2023 at 12:06, Isaac Morland <isaac.morland@gmail.com>
wrote:

Thanks everybody. So based on the latest discussion I will:

1) rename the column from “Source code” to “Internal name”; and
2) change the contents to NULL except when the language (identified by
oid) is INTERNAL or C.

Patch forthcoming, I hope.

I've attached a patch for this. It turns out to simplify the existing code
in one way because the recently added call to pg_get_function_sqlbody() is
no longer needed since it applies only to SQL functions, which will now
display as a blank column.

I implemented the change and was surprised to see that no tests failed.
Turns out that while there are several tests for \df, there were none for
\df+. I added a couple, just using \df+ on some functions that appear to me
to be present on plain vanilla Postgres.

I was initially concerned about translation support for the column
heading, but it turns out that \dT+ already has a column with the exact
same name so it appears we don’t need any new translations.

I welcome comments and feedback. Now to try to find something manageable
to review.

looks well

you miss update psql documentation

https://www.postgresql.org/docs/current/app-psql.html

If the form \df+ is used, additional information about each function is
shown, including volatility, parallel safety, owner, security
classification, access privileges, language, source code and description.

you should to assign your patch to commitfest app

https://commitfest.postgresql.org/

Regards

Pavel

On Wed, 18 Jan 2023 at 00:00, Pavel Stehule <pavel.stehule@gmail.com> wrote:

út 17. 1. 2023 v 20:29 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

I welcome comments and feedback. Now to try to find something manageable
to review.

looks well

you miss update psql documentation

https://www.postgresql.org/docs/current/app-psql.html

If the form \df+ is used, additional information about each function is
shown, including volatility, parallel safety, owner, security
classification, access privileges, language, source code and description.

Thanks, and sorry about that, it just completely slipped my mind. I’ve
attached a revised patch with a slight update of the psql documentation.

you should to assign your patch to commitfest app

https://commitfest.postgresql.org/

I thought I had: https://commitfest.postgresql.org/42/4133/

Did I miss something?

st 18. 1. 2023 v 16:27 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

On Wed, 18 Jan 2023 at 00:00, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

út 17. 1. 2023 v 20:29 odesílatel Isaac Morland <isaac.morland@gmail.com>
napsal:

I welcome comments and feedback. Now to try to find something manageable
to review.

looks well

you miss update psql documentation

https://www.postgresql.org/docs/current/app-psql.html

If the form \df+ is used, additional information about each function is
shown, including volatility, parallel safety, owner, security
classification, access privileges, language, source code and description.

Thanks, and sorry about that, it just completely slipped my mind. I’ve
attached a revised patch with a slight update of the psql documentation.

you should to assign your patch to commitfest app

https://commitfest.postgresql.org/

I thought I had: https://commitfest.postgresql.org/42/4133/

ok

Did I miss something?

it looks well

regards

Pavel

On Wed, Jan 18, 2023 at 10:27:46AM -0500, Isaac Morland wrote:

On Wed, 18 Jan 2023 at 00:00, Pavel Stehule <pavel.stehule@gmail.com> wrote:

út 17. 1. 2023 v 20:29 odesílatel Isaac Morland <isaac.morland@gmail.com> napsal:

I welcome comments and feedback. Now to try to find something manageable
to review.

looks well

you miss update psql documentation

https://www.postgresql.org/docs/current/app-psql.html

If the form \df+ is used, additional information about each function is
shown, including volatility, parallel safety, owner, security
classification, access privileges, language, source code and description.

Thanks, and sorry about that, it just completely slipped my mind. I’ve
attached a revised patch with a slight update of the psql documentation.

you should to assign your patch to commitfest app

https://commitfest.postgresql.org/

I thought I had: https://commitfest.postgresql.org/42/4133/

This is failing tests:
http://cfbot.cputube.org/isaac-morland.html

It seems like any "make check" would fail .. but did you also try
cirrusci from your own github account?
./src/tools/ci/README

BTW, I think "ELSE NULL" could be omitted.

--
Justin

On Thu, 19 Jan 2023 at 11:30, Justin Pryzby <pryzby@telsasoft.com> wrote:

On Wed, Jan 18, 2023 at 10:27:46AM -0500, Isaac Morland wrote:

I thought I had: https://commitfest.postgresql.org/42/4133/

This is failing tests:
http://cfbot.cputube.org/isaac-morland.html

It seems like any "make check" would fail .. but did you also try
cirrusci from your own github account?
./src/tools/ci/README

I definitely ran "make check" but I did not realize there is also cirrusci.
I will look at that. I'm having trouble interpreting the cfbot page to
which you linked but I'll try to run cirrusci myself before worrying too
much about that.

Running "make check" the first time I was surprised to see no failures - so
I added tests for \df+, which passed when I did "make check".

BTW, I think "ELSE NULL" could be omitted.

Looks like; I'll update. Might as well reduce the visual size of the code.