My first patch! (to \df output)

Started by Jon Erdmanabout 13 years ago18 messages

postgresql@thewickedtribe.net

about 13 years ago

1 attachment(s)

Hello Hackers!

So, currently the only way to see if a function is security definer or not is to directly query pg_proc. This is both irritating, and I think perhaps dangerous since security definer functions can be so powerful. I thought that rectifying that would make an excellent first patch, and I was bored today here in Prague since pgconf.eu is now over...so here it is. :)

This patch adds a column to the output of \df titled "Security" with values of "definer" or "invoker" based on the boolean secdef column from pg_proc. I've also included a small doc patch to match. This patch is against master from git. Comments welcome!

I just realized I didn't address regression tests, so I guess this is not actually complete yet. I should have time for that next week after I get back to the states.

I would also like to start discussion about perhaps adding a couple more things to \df+, specifically function execution permissions (which are also exposed nowhere outside the catalog to my knowledge), and maybe search_path since that's related to secdef. Thoughts?

This was actually kind of anti-climactic, since it only took about 5 minutes to make the change and get it working. Didn't really feel the way I expected it to ;)

Attachments:

describe.patchapplication/octet-stream; name=describe.patchDownload

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 6df42fb..d97e5fd 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -245,7 +245,11 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
                          "  WHEN p.proiswindow THEN '%s'\n"
                          "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
                          "  ELSE '%s'\n"
-                         "END as \"%s\"",
+                         "END as \"%s\",\n"
+                         "  CASE\n"
+                         "    WHEN prosecdef THEN 'definer'\n"
+                         "    ELSE 'invoker'\n"
+                         "  END AS \"%s\"\n",
                          gettext_noop("Result data type"),
                          gettext_noop("Argument data types"),
        /* translator: "agg" is short for "aggregate" */
@@ -253,7 +257,8 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
                          gettext_noop("window"),
                          gettext_noop("trigger"),
                          gettext_noop("normal"),
-                         gettext_noop("Type"));
+                         gettext_noop("Type"),
+                         gettext_noop("Security"));
    else if (pset.sversion >= 80100)
        appendPQExpBuffer(&buf,
                     "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
@@ -291,14 +296,19 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
                          "    WHEN p.proisagg THEN '%s'\n"
                          "    WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s                          "    WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
                          "    ELSE '%s'\n"
-                         "  END AS \"%s\"",
+                         "  END AS \"%s\",\n"
+                         "  CASE\n"
+                         "    WHEN prosecdef THEN 'definer'\n"
+                         "    ELSE 'invoker'\n"
+                         "  END AS \"%s\"\n",
                          gettext_noop("Result data type"),
                          gettext_noop("Argument data types"),
        /* translator: "agg" is short for "aggregate" */
                          gettext_noop("agg"),
                          gettext_noop("trigger"),
                          gettext_noop("normal"),
-                         gettext_noop("Type"));
+                         gettext_noop("Type"),
+                         gettext_noop("Security"));
    else
        appendPQExpBuffer(&buf,
                     "  CASE WHEN p.proretset THEN 'SETOF ' ELSE '' END ||\n"
@@ -308,14 +318,19 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
                          "    WHEN p.proisagg THEN '%s'\n"
                          "    WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
                          "    ELSE '%s'\n"
-                         "  END AS \"%s\"",
+                         "  END AS \"%s\",\n"
+                         "  CASE\n"
+                         "    WHEN prosecdef THEN 'definer'\n"
+                         "    ELSE 'invoker'\n"
+                         "  END AS \"%s\"\n",
                          gettext_noop("Result data type"),
                          gettext_noop("Argument data types"),
        /* translator: "agg" is short for "aggregate" */
                          gettext_noop("agg"),
                          gettext_noop("trigger"),
                          gettext_noop("normal"),
-                         gettext_noop("Type"));
+                         gettext_noop("Type"),
+                         gettext_noop("Security"));
 
    if (verbose)
        appendPQExpBuffer(&buf,

Pavel Stehule

pavel.stehule@gmail.com

about 13 years ago

In reply to: Jon Erdman (#1)

Re: My first patch! (to \df output)

Hello

2012/10/27 Jon Erdman <postgresql@thewickedtribe.net>:

Hello Hackers!

So, currently the only way to see if a function is security definer or not is to directly query pg_proc. This is both irritating, and I think perhaps dangerous since security definer functions can be so powerful. I thought that rectifying that would make an excellent first patch, and I was bored today here in Prague since pgconf.eu is now over...so here it is. :)

This patch adds a column to the output of \df titled "Security" with values of "definer" or "invoker" based on the boolean secdef column from pg_proc. I've also included a small doc patch to match. This patch is against master from git. Comments welcome!

I just realized I didn't address regression tests, so I guess this is not actually complete yet. I should have time for that next week after I get back to the states.

I would also like to start discussion about perhaps adding a couple more things to \df+, specifically function execution permissions (which are also exposed nowhere outside the catalog to my knowledge), and maybe search_path since that's related to secdef. Thoughts?

I prefer show this in \dt+ for column "Security" - and for other
functionality maybe new statement.

This was actually kind of anti-climactic, since it only took about 5 minutes to make the change and get it working. Didn't really feel the way I expected it to ;)

:) yes, hacking is funny

Regards

Pavel

Show quoted text

--
Jon T Erdman
Postgresql Zealot

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Greg Sabino Mullane

greg@turnstep.com

about 13 years ago

In reply to: Jon Erdman (#1)

Re: My first patch! (to \df output)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

This was actually kind of anti-climactic, since it only
took about 5 minutes to make the change and get it
working. Didn't really feel the way I expected it to ;)

Well, we can reject your patch and start bike-shedding
it for the next four months, if that makes you feel better! :)

Congrats!

- --
Greg Sabino Mullane greg@turnstep.com
PGP Key: 0x14964AC8 201210271914
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAlCMau4ACgkQvJuQZxSWSshdoQCg6eJ14LLcJrn04rN2/efO14iz
swgAoPbBSv8PAre6qtVrRH3LL/iNQqeD
=m/ns
-----END PGP SIGNATURE-----

Jon Erdman

postgresql@thewickedtribe.net

about 13 years ago

In reply to: Pavel Stehule (#2)

1 attachment(s)

Re: My first patch! (to \df output)

On Oct 27, 2012, at 10:45 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hello

2012/10/27 Jon Erdman <postgresql@thewickedtribe.net>:

Hello Hackers!

So, currently the only way to see if a function is security definer or not is to directly query pg_proc. This is both irritating, and I think perhaps dangerous since security definer functions can be so powerful. I thought that rectifying that would make an excellent first patch, and I was bored today here in Prague since pgconf.eu is now over...so here it is. :)

This patch adds a column to the output of \df titled "Security" with values of "definer" or "invoker" based on the boolean secdef column from pg_proc. I've also included a small doc patch to match. This patch is against master from git. Comments welcome!

I just realized I didn't address regression tests, so I guess this is not actually complete yet. I should have time for that next week after I get back to the states.

I would also like to start discussion about perhaps adding a couple more things to \df+, specifically function execution permissions (which are also exposed nowhere outside the catalog to my knowledge), and maybe search_path since that's related to secdef. Thoughts?

I prefer show this in \dt+ for column "Security" - and for other
functionality maybe new statement.

I'm assuming you meant "\df+", and I've changed it accordingly. With this change there is now nothing to change in the regression tests, so please consider this my formal and complete submission.

Attachments:

describe.patchapplication/octet-stream; name=describe.patchDownload

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index c41593c..04bde18 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1178,9 +1178,10 @@ testdb=&gt;
 
         <listitem>
         <para>
-        Lists functions, together with their arguments, return types, and
-        function types, which are classified as <quote>agg</> (aggregate),
-        <quote>normal</>, <quote>trigger</>, or <quote>window</>.
+        Lists functions, together with their arguments, return types,
+        function types, and security, which are classified as 
+        <quote>agg</> (aggregate), <quote>normal</>, <quote>trigger</>, 
+        or <quote>window</>.
         To display only functions
         of specific type(s), add the corresponding letters <literal>a</>,
         <literal>n</>, <literal>t</>, or <literal>w</> to the command.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 15d02ee..5e5cefe 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -254,7 +254,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 						  "  WHEN p.proiswindow THEN '%s'\n"
 						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
 						  "  ELSE '%s'\n"
-						  "END as \"%s\"",
+						  " END as \"%s\"",
 						  gettext_noop("Result data type"),
 						  gettext_noop("Argument data types"),
 		/* translator: "agg" is short for "aggregate" */
@@ -329,14 +329,19 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool
 	if (verbose)
 		appendPQExpBuffer(&buf,
 						  ",\n CASE\n"
+						  "  WHEN prosecdef THEN 'definer'\n"
+						  "  ELSE 'invoker'\n"
+						  " END AS \"%s\""
+						  ",\n CASE\n"
 						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
 						  "  WHEN p.provolatile = 's' THEN '%s'\n"
 						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
-						  "END as \"%s\""
+						  " END as \"%s\""
 				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
 						  "  l.lanname as \"%s\",\n"
 						  "  p.prosrc as \"%s\",\n"
 				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
+						  gettext_noop("Security"),
 						  gettext_noop("immutable"),
 						  gettext_noop("stable"),
 						  gettext_noop("volatile"),

Jon Erdman

postgresql@thewickedtribe.net

about 13 years ago

In reply to: Jon Erdman (#4)

1 attachment(s)

Re: My first patch! (to \df output)

Oops! Here it is in the proper diff format. I didn't have my env set up correctly :(

Attachments:

describe.patchapplication/octet-stream; name=describe.patchDownload

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index c41593c..04bde18
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=&gt;
*** 1178,1186 ****
  
          <listitem>
          <para>
!         Lists functions, together with their arguments, return types, and
!         function types, which are classified as <quote>agg</> (aggregate),
!         <quote>normal</>, <quote>trigger</>, or <quote>window</>.
          To display only functions
          of specific type(s), add the corresponding letters <literal>a</>,
          <literal>n</>, <literal>t</>, or <literal>w</> to the command.
--- 1178,1187 ----
  
          <listitem>
          <para>
!         Lists functions, together with their arguments, return types,
!         function types, and security, which are classified as 
!         <quote>agg</> (aggregate), <quote>normal</>, <quote>trigger</>, 
!         or <quote>window</>.
          To display only functions
          of specific type(s), add the corresponding letters <literal>a</>,
          <literal>n</>, <literal>t</>, or <literal>w</> to the command.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 15d02ee..5e5cefe
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeFunctions(const char *functypes,
*** 254,260 ****
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  "END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
--- 254,260 ----
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  " END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
*************** describeFunctions(const char *functypes,
*** 329,342 ****
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  "END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),
--- 329,347 ----
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
+ 						  "  WHEN prosecdef THEN 'definer'\n"
+ 						  "  ELSE 'invoker'\n"
+ 						  " END AS \"%s\""
+ 						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  " END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
+ 						  gettext_noop("Security"),
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),

Robert Haas

robertmhaas@gmail.com

about 13 years ago

In reply to: Jon Erdman (#5)

Re: My first patch! (to \df output)

On Fri, Nov 9, 2012 at 3:22 PM, Jon Erdman
<postgresql@thewickedtribe.net> wrote:

Oops! Here it is in the proper diff format. I didn't have my env set up correctly :(

Better add it here so it doesn't get lost:

https://commitfest.postgresql.org/action/commitfest_view/open

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Stephen Frost

sfrost@snowman.net

about 13 years ago

In reply to: Jon Erdman (#5)

Re: My first patch! (to \df output)

* Jon Erdman (postgresql@thewickedtribe.net) wrote:

Oops! Here it is in the proper diff format. I didn't have my env set up correctly :(

No biggie, and to get the bike-shedding started, I don't really like the
column name or the values.. :) I feel like something clearer would be
"Runs_As" with "caller" or "owner".. Saying "Security" makes me think
of ACLs more than what user ID the function runs as, to be honest.

Looking at the actual patch itself, it looks like you have some
unecessary whitespace changes included..?

Thanks!

Stephen

Phil Sorber

phil@omniti.com

almost 13 years ago

In reply to: Stephen Frost (#7)

Re: My first patch! (to \df output)

On Sat, Dec 29, 2012 at 1:56 PM, Stephen Frost <sfrost@snowman.net> wrote:

* Jon Erdman (postgresql@thewickedtribe.net) wrote:

Oops! Here it is in the proper diff format. I didn't have my env set up correctly :(

No biggie, and to get the bike-shedding started, I don't really like the
column name or the values.. :) I feel like something clearer would be
"Runs_As" with "caller" or "owner".. Saying "Security" makes me think
of ACLs more than what user ID the function runs as, to be honest.

Looking at the actual patch itself, it looks like you have some
unecessary whitespace changes included..?

Thanks!

Stephen

Stephen, I think Jon's column name and values make a lot of sense.
That being said, I do agree with your point of making it clearer for
the person viewing the output, I just don't know if it would be
confusing when they wanted to change it or were trying to understand
how it related.

Agree on the extra spaces in the docs.

Jon, I think you inserted your changes improperly in the docs. The
classifications apply to the type, not to security.

Also, you need to use the %s place holder and the gettext_noop() call
for your values as well as your column name.

Compiles and tests ok. Results look as expected.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jon Erdman

postgresql@thewickedtribe.net

almost 13 years ago

In reply to: Phil Sorber (#8)

Re: My first patch! (to \df output)

I did realize that since I moved it to + the doc should change, but I didn't address that. I'll get on it this weekend.

As far as the column name and displayed values go, they're taken from the CREATE FUNCTION syntax, and were recommended by Magnus, Bruce, and Fetter, who were all sitting next to me day after pgconf.eu Prague. I personally have no strong feelings either way, I just want to be able to see the info without having to directly query pg_proc. Whatever you all agree on is fine by me.
--
Jon T Erdman

Chief Information Officer voice: (312) 285-6735
Progressive Practice, Inc. jon@progressivepractice.com
P.O. Box 17288 www.progressivepractice.com
Rochester, NY 14617

On Jan 18, 2013, at 5:51 PM, Phil Sorber <phil@omniti.com> wrote:

On Sat, Dec 29, 2012 at 1:56 PM, Stephen Frost <sfrost@snowman.net> wrote:

* Jon Erdman (postgresql@thewickedtribe.net) wrote:

Oops! Here it is in the proper diff format. I didn't have my env set up correctly :(

No biggie, and to get the bike-shedding started, I don't really like the
column name or the values.. :) I feel like something clearer would be
"Runs_As" with "caller" or "owner".. Saying "Security" makes me think
of ACLs more than what user ID the function runs as, to be honest.

Looking at the actual patch itself, it looks like you have some
unecessary whitespace changes included..?

Thanks!

Stephen

Stephen, I think Jon's column name and values make a lot of sense.
That being said, I do agree with your point of making it clearer for
the person viewing the output, I just don't know if it would be
confusing when they wanted to change it or were trying to understand
how it related.

Agree on the extra spaces in the docs.

Jon, I think you inserted your changes improperly in the docs. The
classifications apply to the type, not to security.

Also, you need to use the %s place holder and the gettext_noop() call
for your values as well as your column name.

Compiles and tests ok. Results look as expected.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Phil Sorber

phil@omniti.com

almost 13 years ago

In reply to: Jon Erdman (#9)

Re: My first patch! (to \df output)

On Jan 19, 2013 10:55 AM, "Jon Erdman" <postgresql@thewickedtribe.net>
wrote:

I did realize that since I moved it to + the doc should change, but I

didn't address that. I'll get on it this weekend.

As far as the column name and displayed values go, they're taken from the

CREATE FUNCTION syntax, and were recommended by Magnus, Bruce, and Fetter,
who were all sitting next to me day after pgconf.eu Prague. I personally
have no strong feelings either way, I just want to be able to see the info
without having to directly query pg_proc. Whatever you all agree on is fine
by me.

Sounds like you have a +4/-1 on the names then. I'd keep them as is.

#11

Stephen Frost

sfrost@snowman.net

almost 13 years ago

In reply to: Phil Sorber (#8)

Re: My first patch! (to \df output)

* Phil Sorber (phil@omniti.com) wrote:

Stephen, I think Jon's column name and values make a lot of sense.

a'ight. I can't think of anything better.

Thanks,

Stephen

#12

Jon Erdman

postgresql@thewickedtribe.net

almost 13 years ago

In reply to: Jon Erdman (#1)

2 attachment(s)

Re: My first patch! (to \df output)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Updated the patch in commitfest with the doc change, and added a
comment to explain the whitespace change (it was to clean up the sql
indentation). I've also attached the new patch here for reference.
- --
Jon T Erdman (aka StuckMojo)
PostgreSQL Zealot

On 01/20/2013 08:27 PM, Craig Ringer wrote:

On 01/19/2013 11:54 PM, Jon Erdman wrote:

I did realize that since I moved it to + the doc should change,
but I didn't address that. I'll get on it this weekend.

Held as waiting on author, then. Please update
https://commitfest.postgresql.org/action/patch_view?id=1008 when
you post a new revision.

-- Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlD/cNYACgkQRAk1+p0GhSGwJQCfa+8SbL9cYHZkqfmlRlgqcXf9
qD4AnjSZwSXQmOMK8thSs6CdiDxQkJCJ
=H+km
-----END PGP SIGNATURE-----

Attachments:

describe.uni.patchtext/x-patch; name=describe.uni.patchDownload

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index c41593c..0eb6b63
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=&gt;
*** 1188,1194 ****
          class="parameter">pattern</replaceable> is specified, only
          functions whose names match the pattern are shown.  If the
          form <literal>\df+</literal> is used, additional information
!         about each function, including volatility, language, source
          code and description, is shown.  By default, only user-created
          objects are shown; supply a pattern or the <literal>S</literal>
          modifier to include system objects.
--- 1188,1194 ----
          class="parameter">pattern</replaceable> is specified, only
          functions whose names match the pattern are shown.  If the
          form <literal>\df+</literal> is used, additional information
!         about each function, including security, volatility, language, source
          code and description, is shown.  By default, only user-created
          objects are shown; supply a pattern or the <literal>S</literal>
          modifier to include system objects.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 15d02ee..5e5cefe
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeFunctions(const char *functypes,
*** 254,260 ****
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  "END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
--- 254,260 ----
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  " END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
*************** describeFunctions(const char *functypes,
*** 329,342 ****
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  "END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),
--- 329,347 ----
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
+ 						  "  WHEN prosecdef THEN 'definer'\n"
+ 						  "  ELSE 'invoker'\n"
+ 						  " END AS \"%s\""
+ 						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  " END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
+ 						  gettext_noop("Security"),
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),

0x9D068521.ascapplication/pgp-keys; name=0x9D068521.ascDownload

Import Notes

Reply to msg id not found: 50FCA792.90602@2ndQuadrant.com

#13

Phil Sorber

phil@omniti.com

almost 13 years ago

In reply to: Jon Erdman (#12)

Re: My first patch! (to \df output)

On Wed, Jan 23, 2013 at 12:10 AM, Jon Erdman
<postgresql@thewickedtribe.net> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Updated the patch in commitfest with the doc change, and added a
comment to explain the whitespace change (it was to clean up the sql
indentation). I've also attached the new patch here for reference.

Docs looks good. Spaces gone.

Still need to replace 'definer' and 'invoker' with %s and add the
corresponding gettext_noop() calls I think.

- --
Jon T Erdman (aka StuckMojo)
PostgreSQL Zealot

On 01/20/2013 08:27 PM, Craig Ringer wrote:

On 01/19/2013 11:54 PM, Jon Erdman wrote:

I did realize that since I moved it to + the doc should change,
but I didn't address that. I'll get on it this weekend.

Held as waiting on author, then. Please update
https://commitfest.postgresql.org/action/patch_view?id=1008 when
you post a new revision.

-- Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlD/cNYACgkQRAk1+p0GhSGwJQCfa+8SbL9cYHZkqfmlRlgqcXf9
qD4AnjSZwSXQmOMK8thSs6CdiDxQkJCJ
=H+km
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14

Jon Erdman

postgresql@thewickedtribe.net

almost 13 years ago

In reply to: Phil Sorber (#13)

2 attachment(s)

Re: My first patch! (to \df output)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Done. Attached.
- --
Jon T Erdman (aka StuckMojo)
PostgreSQL Zealot

On 01/22/2013 11:17 PM, Phil Sorber wrote:

On Wed, Jan 23, 2013 at 12:10 AM, Jon Erdman
<postgresql@thewickedtribe.net> wrote:

Updated the patch in commitfest with the doc change, and added a
comment to explain the whitespace change (it was to clean up the
sql indentation). I've also attached the new patch here for
reference.

Docs looks good. Spaces gone.

Still need to replace 'definer' and 'invoker' with %s and add
the corresponding gettext_noop() calls I think.

-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlD/dcoACgkQRAk1+p0GhSEKHQCZAW8UNqSjYxBgBvt2nuffrkrV
+9AAn1hChpY5Jg8G8T3XmlIb+3VUSEQ2
=3cFD
-----END PGP SIGNATURE-----

Attachments:

describe.uni.patchtext/x-patch; name=describe.uni.patchDownload

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index c41593c..0eb6b63
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** testdb=&gt;
*** 1188,1194 ****
          class="parameter">pattern</replaceable> is specified, only
          functions whose names match the pattern are shown.  If the
          form <literal>\df+</literal> is used, additional information
!         about each function, including volatility, language, source
          code and description, is shown.  By default, only user-created
          objects are shown; supply a pattern or the <literal>S</literal>
          modifier to include system objects.
--- 1188,1194 ----
          class="parameter">pattern</replaceable> is specified, only
          functions whose names match the pattern are shown.  If the
          form <literal>\df+</literal> is used, additional information
!         about each function, including security, volatility, language, source
          code and description, is shown.  By default, only user-created
          objects are shown; supply a pattern or the <literal>S</literal>
          modifier to include system objects.
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 15d02ee..3dadc09
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeFunctions(const char *functypes,
*** 254,260 ****
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  "END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
--- 254,260 ----
  						  "  WHEN p.proiswindow THEN '%s'\n"
  						  "  WHEN p.prorettype = 'pg_catalog.trigger'::pg_catalog.regtype THEN '%s'\n"
  						  "  ELSE '%s'\n"
! 						  " END as \"%s\"",
  						  gettext_noop("Result data type"),
  						  gettext_noop("Argument data types"),
  		/* translator: "agg" is short for "aggregate" */
*************** describeFunctions(const char *functypes,
*** 329,342 ****
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  "END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),
--- 329,349 ----
  	if (verbose)
  		appendPQExpBuffer(&buf,
  						  ",\n CASE\n"
+ 						  "  WHEN prosecdef THEN '%s'\n"
+ 						  "  ELSE '%s'\n"
+ 						  " END AS \"%s\""
+ 						  ",\n CASE\n"
  						  "  WHEN p.provolatile = 'i' THEN '%s'\n"
  						  "  WHEN p.provolatile = 's' THEN '%s'\n"
  						  "  WHEN p.provolatile = 'v' THEN '%s'\n"
! 						  " END as \"%s\""
  				   ",\n  pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n"
  						  "  l.lanname as \"%s\",\n"
  						  "  p.prosrc as \"%s\",\n"
  				  "  pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"",
+ 						  gettext_noop("definer"),
+ 						  gettext_noop("invoker"),
+ 						  gettext_noop("Security"),
  						  gettext_noop("immutable"),
  						  gettext_noop("stable"),
  						  gettext_noop("volatile"),

0x9D068521.ascapplication/pgp-keys; name=0x9D068521.ascDownload

#15

Phil Sorber

phil@omniti.com

almost 13 years ago

In reply to: Jon Erdman (#14)

Re: My first patch! (to \df output)

On Wed, Jan 23, 2013 at 12:31 AM, Jon Erdman
<postgresql@thewickedtribe.net> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Done. Attached.
- --
Jon T Erdman (aka StuckMojo)
PostgreSQL Zealot

On 01/22/2013 11:17 PM, Phil Sorber wrote:

On Wed, Jan 23, 2013 at 12:10 AM, Jon Erdman
<postgresql@thewickedtribe.net> wrote:

Updated the patch in commitfest with the doc change, and added a
comment to explain the whitespace change (it was to clean up the
sql indentation). I've also attached the new patch here for
reference.

Docs looks good. Spaces gone.

Still need to replace 'definer' and 'invoker' with %s and add
the corresponding gettext_noop() calls I think.

This looks good to me now. Compiles and works as described.

One thing I would note for the future though, when updating a patch,
add a version to the file name to distinguish it from older versions
of the patch.

-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlD/dcoACgkQRAk1+p0GhSEKHQCZAW8UNqSjYxBgBvt2nuffrkrV
+9AAn1hChpY5Jg8G8T3XmlIb+3VUSEQ2
=3cFD
-----END PGP SIGNATURE-----

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16

Craig Ringer

craig@2ndQuadrant.com

almost 13 years ago

In reply to: Phil Sorber (#15)

Re: My first patch! (to \df output)

On 01/24/2013 01:50 AM, Phil Sorber wrote:

This looks good to me now. Compiles and works as described.

Ready to go?

https://commitfest.postgresql.org/action/patch_view?id=1008

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

#17

Phil Sorber

phil@omniti.com

almost 13 years ago

In reply to: Craig Ringer (#16)

Re: My first patch! (to \df output)

On Thu, Jan 24, 2013 at 2:27 AM, Craig Ringer <craig@2ndquadrant.com> wrote:

On 01/24/2013 01:50 AM, Phil Sorber wrote:

This looks good to me now. Compiles and works as described.

Ready to go?

https://commitfest.postgresql.org/action/patch_view?id=1008

I guess I wasn't ready to be so bold, but sure. :) I changed it to
'ready for committer'.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18

Heikki Linnakangas

hlinnakangas@vmware.com

almost 13 years ago

In reply to: Jon Erdman (#14)

Re: My first patch! (to \df output)

On 23.01.2013 07:31, Jon Erdman wrote:

Done. Attached.

Thanks, committed.

On 29.12.2012 20:56, Stephen Frost wrote:

No biggie, and to get the bike-shedding started, I don't really like the
column name or the values.. :) I feel like something clearer would be
"Runs_As" with "caller" or "owner".. Saying "Security" makes me think
of ACLs more than what user ID the function runs as, to be honest.

I have to agree that calling the property "security definer/invoker" is
a poor name in general. "security" is such on overloaded word that it
could mean anything. "Run as" would make a lot more sense. But given
that that's the nomenclature we have in the CREATE FUNCTION statement,
the docs, prosecdef column name and everywhere, that's what we have to
call it in \df+ too.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers