Thread: Can pg_dump make use of CURRENT/SESSION_USER

Are there any use-cases for pg_dump to use CURRENT/SESSION_USER in its
output, so that restores will not be hard-coded to the dump user?  I
didn't see any cases of that, but wanted to ask.

pg_dump doesn't have to restore into old clusters so there isn't a
problem with backward compatibility.

---------------------------------------------------------------------------

----- Forwarded message from Alvaro Herrera <alvherre@alvh.no-ip.org> -----

Date: Mon, 09 Mar 2015 18:46:02 +0000
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: pgsql-committers@postgresql.org
Subject: [COMMITTERS] pgsql: Allow CURRENT/SESSION_USER to be used in certain commands

Allow CURRENT/SESSION_USER to be used in certain commands

Commands such as ALTER USER, ALTER GROUP, ALTER ROLE, GRANT, and the
various ALTER OBJECT / OWNER TO, as well as ad-hoc clauses related to
roles such as the AUTHORIZATION clause of CREATE SCHEMA, the FOR clause
of CREATE USER MAPPING, and the FOR ROLE clause of ALTER DEFAULT
PRIVILEGES can now take the keywords CURRENT_USER and SESSION_USER as
user specifiers in place of an explicit user name.

This commit also fixes some quite ugly handling of special standards-
mandated syntax in CREATE USER MAPPING, which in particular would fail
to work in presence of a role named "current_user".

The special role specifiers PUBLIC and NONE also have more consistent
handling now.

Also take the opportunity to add location tracking to user specifiers.

Authors: Kyotaro Horiguchi.  Heavily reworked by Álvaro Herrera.
Reviewed by: Rushabh Lathia, Adam Brightwell, Marti Raudsepp.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/31eae6028eca4365e7165f5f33fee1ed0486aee0

Modified Files
--------------
doc/src/sgml/ref/alter_aggregate.sgml            |    3 +-
doc/src/sgml/ref/alter_collation.sgml            |    2 +-
doc/src/sgml/ref/alter_conversion.sgml           |    2 +-
doc/src/sgml/ref/alter_database.sgml             |    2 +-
doc/src/sgml/ref/alter_domain.sgml               |    2 +-
doc/src/sgml/ref/alter_event_trigger.sgml        |    2 +-
doc/src/sgml/ref/alter_foreign_data_wrapper.sgml |    2 +-
doc/src/sgml/ref/alter_foreign_table.sgml        |    2 +-
doc/src/sgml/ref/alter_function.sgml             |    2 +-
doc/src/sgml/ref/alter_group.sgml                |   10 +-
doc/src/sgml/ref/alter_language.sgml             |    2 +-
doc/src/sgml/ref/alter_large_object.sgml         |    2 +-
doc/src/sgml/ref/alter_materialized_view.sgml    |    2 +-
doc/src/sgml/ref/alter_opclass.sgml              |   11 +-
doc/src/sgml/ref/alter_operator.sgml             |    7 +-
doc/src/sgml/ref/alter_opfamily.sgml             |   19 +-
doc/src/sgml/ref/alter_role.sgml                 |   35 +-
doc/src/sgml/ref/alter_schema.sgml               |    2 +-
doc/src/sgml/ref/alter_sequence.sgml             |    2 +-
doc/src/sgml/ref/alter_server.sgml               |    2 +-
doc/src/sgml/ref/alter_table.sgml                |    2 +-
doc/src/sgml/ref/alter_tablespace.sgml           |    2 +-
doc/src/sgml/ref/alter_tsconfig.sgml             |    2 +-
doc/src/sgml/ref/alter_tsdictionary.sgml         |    2 +-
doc/src/sgml/ref/alter_type.sgml                 |    2 +-
doc/src/sgml/ref/alter_user.sgml                 |   16 +-
doc/src/sgml/ref/alter_user_mapping.sgml         |    2 +-
doc/src/sgml/ref/alter_view.sgml                 |    2 +-
doc/src/sgml/ref/create_schema.sgml              |   14 +-
doc/src/sgml/ref/grant.sgml                      |   33 +-
src/backend/catalog/aclchk.c                     |   54 +-
src/backend/commands/alter.c                     |    2 +-
src/backend/commands/extension.c                 |    2 +-
src/backend/commands/foreigncmds.c               |   58 +-
src/backend/commands/policy.c                    |   18 +-
src/backend/commands/schemacmds.c                |   21 +-
src/backend/commands/tablecmds.c                 |    4 +-
src/backend/commands/tablespace.c                |    2 +-
src/backend/commands/user.c                      |  116 +--
src/backend/nodes/copyfuncs.c                    |   49 +-
src/backend/nodes/equalfuncs.c                   |   45 +-
src/backend/parser/gram.y                        |  228 ++++--
src/backend/parser/parse_utilcmd.c               |    4 +-
src/backend/utils/adt/acl.c                      |  116 ++-
src/include/commands/user.h                      |    2 +-
src/include/nodes/nodes.h                        |    1 +
src/include/nodes/parsenodes.h                   |   50 +-
src/include/utils/acl.h                          |    8 +-
src/test/regress/expected/rolenames.out          |  940 ++++++++++++++++++++++
src/test/regress/parallel_schedule               |    2 +-
src/test/regress/serial_schedule                 |    1 +
src/test/regress/sql/rolenames.sql               |  434 ++++++++++
52 files changed, 2000 insertions(+), 347 deletions(-)


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

----- End forwarded message -----

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +

Bruce Momjian wrote:
> 
> Are there any use-cases for pg_dump to use CURRENT/SESSION_USER in its
> output, so that restores will not be hard-coded to the dump user?  I
> didn't see any cases of that, but wanted to ask.

Good question.  I don't know, probably not.

If we ever implement something like

COMMENT ON CURRENT_DATABASE IS ...

it will be useful, because you will be able to restore a dump into
another database and have the comment apply to the target database.
(Also, I wonder about
ALTER USER foo IN DATABASE current_database ...
because that will let us dump per-database user options too.)


-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Mar 17, 2015 at 1:24 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> If we ever implement something like
>
> COMMENT ON CURRENT_DATABASE IS ...
>
> it will be useful, because you will be able to restore a dump into
> another database and have the comment apply to the target database.
> (Also, I wonder about
> ALTER USER foo IN DATABASE current_database ...
> because that will let us dump per-database user options too.)

+1 for both of those ideas.

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

<div dir="ltr"><div class="gmail_extra"><br />On Wed, Mar 18, 2015 at 1:24 PM, Robert Haas <<a
href="mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</a>>wrote:<br />><br />> On Tue, Mar 17, 2015 at
1:24PM, Alvaro Herrera<br />> <<a href="mailto:alvherre@2ndquadrant.com">alvherre@2ndquadrant.com</a>>
wrote:<br/>> > If we ever implement something like<br />> ><br />> > COMMENT ON CURRENT_DATABASE IS
...<br/>> ><br />> > it will be useful, because you will be able to restore a dump into<br />> >
anotherdatabase and have the comment apply to the target database.<br /><br /></div><div class="gmail_extra">I think
it'ssimple to implement, but how about pg_dump... we need to add new option (like --use-current-database) or am I
missingsomething ? <br /></div><div class="gmail_extra"><br /><br />> > (Also, I wonder about<br />> >
ALTERUSER foo IN DATABASE current_database ...<br />> > because that will let us dump per-database user options
too.)<br/>><br />> +1 for both of those ideas.<br />><br /><br /></div><div class="gmail_extra">Can you
explainmore about this idea?<br /></div><div class="gmail_extra"><br /></div><div class="gmail_extra">Regards,<br /><br
/></div><divclass="gmail_extra">--<br />Fabrízio de Royes Mello<br />Consultoria/Coaching PostgreSQL<br />>>
Timbira:<a href="http://www.timbira.com.br">http://www.timbira.com.br</a><br />>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>

On Wed, Mar 18, 2015 at 1:23 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
>> > If we ever implement something like
>> >
>> > COMMENT ON CURRENT_DATABASE IS ...
>> >
>> > it will be useful, because you will be able to restore a dump into
>> > another database and have the comment apply to the target database.
>
> I think it's simple to implement, but how about pg_dump... we need to add
> new option (like --use-current-database) or am I missing something ?

I think we'd just change it to use the new syntax, full stop.  I see
no need for an option.

>> > (Also, I wonder about
>> > ALTER USER foo IN DATABASE current_database ...
>> > because that will let us dump per-database user options too.)
>>
>> +1 for both of those ideas.
>
> Can you explain more about this idea?

Uh, what do you want explained?

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

On Wed, Mar 18, 2015 at 2:09 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
> Just one last doubt. Do we expose a new function called "current_database"
> like "current_catalog", "current_user", ... ?

Why would we do that?

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

Fabrízio de Royes Mello <fabriziomello@gmail.com> writes:
> Just one last doubt. Do we expose a new function called "current_database"
> like "current_catalog", "current_user", ... ?

There already is one.  But that would have nothing to do with the proposed
patch anyway, because the bits of syntax in question are not expressions
and you should not try to turn them into things that would call functions.
        regards, tom lane

<div dir="ltr"><div class="gmail_extra"><br />On Wed, Mar 18, 2015 at 3:13 PM, Robert Haas <<a
href="mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</a>>wrote:<br />><br />> On Wed, Mar 18, 2015 at
2:09PM, Fabrízio de Royes Mello<br />> <<a href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>>
wrote:<br/>> > Just one last doubt. Do we expose a new function called "current_database"<br />> > like
"current_catalog","current_user", ... ?<br />><br />> Why would we do that?<br />><br /><br /></div><div
class="gmail_extra">Wedon't need it... it's another patch.<br /></div><div class="gmail_extra"><br /></div><div
class="gmail_extra">Regards,<br/></div><div class="gmail_extra"><br />--<br />Fabrízio de Royes Mello<br
/>Consultoria/CoachingPostgreSQL<br />>> Timbira: <a
href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>

On Sat, Apr 25, 2015 at 8:05 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
>> >> > If we ever implement something like
>> >> >
>> >> > COMMENT ON CURRENT_DATABASE IS ...
>> >> >
>> >> > it will be useful, because you will be able to restore a dump into
>> >> > another database and have the comment apply to the target database.
>> >
>> > I think it's simple to implement, but how about pg_dump... we need to
>> > add
>> > new option (like --use-current-database) or am I missing something ?
>>
>> I think we'd just change it to use the new syntax, full stop.  I see
>> no need for an option.
>
> I'm returning on this...
>
> What's the reasonable syntaxes?
>
> COMMENT ON CURRENT DATABASE IS 'text';
>
> or
>
> COMMENT ON DATABASE { CURRENT_DATABASE | object_name } IS 'text';

The second one would require making CURRENT_DATABASE a reserved
keyword, and I'm not keen to create any more of those.  I like the
first one.  The other alternative that may be worth considering is:

COMMENT ON CURRENT_DATABASE IS 'text';

That doesn't require making CURRENT_DATABASE a reserved keyword, but
it does require making it a keyword, and it doesn't look very SQL-ish.
Still, we have a bunch of other CURRENT_FOO keywords.

But I'm inclined to stick with your first proposal.

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

<div dir="ltr"><div class="gmail_extra">On Tue, Apr 28, 2015 at 1:07 PM, Fabrízio de Royes Mello <<a
href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>>wrote:<br />><br />><br />> On Tue, Apr
28,2015 at 9:38 AM, Robert Haas <<a href="mailto:robertmhaas@gmail.com">robertmhaas@gmail.com</a>> wrote:<br
/>>><br />> > On Sat, Apr 25, 2015 at 8:05 AM, Fabrízio de Royes Mello<br />> > <<a
href="mailto:fabriziomello@gmail.com">fabriziomello@gmail.com</a>>wrote:<br />> > >> >> > If we
everimplement something like<br />> > >> >> ><br />> > >> >> > COMMENT ON
CURRENT_DATABASEIS ...<br />> > >> >> ><br />> > >> >> > it will be useful,
becauseyou will be able to restore a dump into<br />> > >> >> > another database and have the
commentapply to the target database.<br />> > >> ><br />> > >> > I think it's simple to
implement,but how about pg_dump... we need to<br />> > >> > add<br />> > >> > new option
(like--use-current-database) or am I missing something ?<br />> > >><br />> > >> I think we'd
justchange it to use the new syntax, full stop.  I see<br />> > >> no need for an option.<br />> >
><br/>> > > I'm returning on this...<br />> > ><br />> > > What's the reasonable
syntaxes?<br/>> > ><br />> > > COMMENT ON CURRENT DATABASE IS 'text';<br />> > ><br />>
>> or<br />> > ><br />> > > COMMENT ON DATABASE { CURRENT_DATABASE | object_name } IS
'text';<br/>> ><br />> > The second one would require making CURRENT_DATABASE a reserved<br />> >
keyword,and I'm not keen to create any more of those.  I like the<br />> > first one.  The other alternative that
maybe worth considering is:<br />> ><br />> > COMMENT ON CURRENT_DATABASE IS 'text';<br />> ><br
/>>> That doesn't require making CURRENT_DATABASE a reserved keyword, but<br />> > it does require making
ita keyword, and it doesn't look very SQL-ish.<br />> > Still, we have a bunch of other CURRENT_FOO keywords.<br
/>>><br />> > But I'm inclined to stick with your first proposal.<br />> ><br />><br />>
Attachedthe patch to support "COMMENT ON CURRENT DATABASE IS ..." (including pg_dump).<br />><br />> On my next
sparetime I'll send the "ALTER ROLE ... IN CURRENT DATABASE" patch.<br />><br /><br /></div><div
class="gmail_extra">Lookingat the patch again I realize the code is very ugly, so I'll rework the patch.<br /><br
/></div><divclass="gmail_extra">Regards,<br /></div><div class="gmail_extra"><br />--<br />Fabrízio de Royes Mello<br
/>Consultoria/CoachingPostgreSQL<br />>> Timbira: <a
href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>

Fabrízio de Royes Mello wrote:
> 
> Looking at the patch again I realize the code is very ugly, so I'll rework
> the patch.

Yes, I think get_object_address should figure out what to do with the
representation of CURRENT DATABASE directly, rather than having the
COMMENT code morph from that into a list containing the name of the
current database.  Please see about changing SECURITY LABEL at the same
time, if only to make sure that the representation is sane.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Fabrízio de Royes Mello wrote:

> I have this idea:
> 
> 1) Add an ObjectAddress field to CommentStmt struct an set it in gram.y
> 
> 2) In the CommentObject check if CommentStmt->address is
> InvalidObjectAddress then call get_object_address else use it

For DDL deparsing purposes, it seems important that the affected object
address can be reproduced somehow.  I think pg_get_object_address()
should be considered, as well as the object_address test.

If we do as you propose, we would have to deparse
COMMENT ON CURRENT DATABASE IS 'foo'
as COMMENT ON DATABASE whatever_the_name_is IS 'foo'

which is not a fatal objection but doesn't seem all that great.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Apr 30, 2015 at 2:04 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Fabrízio de Royes Mello wrote:
>
>> I have this idea:
>>
>> 1) Add an ObjectAddress field to CommentStmt struct an set it in gram.y
>>
>> 2) In the CommentObject check if CommentStmt->address is
>> InvalidObjectAddress then call get_object_address else use it
>
> For DDL deparsing purposes, it seems important that the affected object
> address can be reproduced somehow.  I think pg_get_object_address()
> should be considered, as well as the object_address test.
>
> If we do as you propose, we would have to deparse
> COMMENT ON CURRENT DATABASE IS 'foo'
> as
>   COMMENT ON DATABASE whatever_the_name_is IS 'foo'
>
> which is not a fatal objection but doesn't seem all that great.

Seeing no activity in the last couple of months for this patch that
had reviews, it is now marked as returned with feedback.
--
Michael

On 2015-08-25 22:01:51 +0900, Michael Paquier wrote:
> Seeing no activity in the last couple of months for this patch that
> had reviews, it is now marked as returned with feedback.

Fabrizio, you after the above moved the patch to next commitfest,
without a new patch or a additional discussion. Why? Just dragging
patches through commitfest justincreases the work for a number of people
(this round me) without a benefit, so that's really not a good idea.

I'm marking the patch as returned with feedback again. Once there's a
new patch we can deal with it in the appropriate commitfest.

Greetings,

Andres Freund

<div dir="ltr"><div class="gmail_extra"><br />On Thu, Sep 3, 2015 at 8:16 AM, Andres Freund <<a
href="mailto:andres@anarazel.de">andres@anarazel.de</a>>wrote:<br />><br />> On 2015-08-25 22:01:51 +0900,
MichaelPaquier wrote:<br />> > Seeing no activity in the last couple of months for this patch that<br />> >
hadreviews, it is now marked as returned with feedback.<br />><br />> Fabrizio, you after the above moved the
patchto next commitfest,<br />> without a new patch or a additional discussion. Why? Just dragging<br />> patches
throughcommitfest justincreases the work for a number of people<br />> (this round me) without a benefit, so that's
reallynot a good idea.<br />><br /><br /></div><div class="gmail_extra">My intention was move to next commitfest and
finishthe "rework", but I didn't finish it due some other professional priorities.<br /><br /></div><div
class="gmail_extra">Sorryby the noise.<br /></div><div class="gmail_extra"><br /><br />> I'm marking the patch as
returnedwith feedback again. Once there's a<br />> new patch we can deal with it in the appropriate commitfest.<br
/>><br/><br /></div><div class="gmail_extra">Ok.<br /></div><div class="gmail_extra"><br /><br /></div><div
class="gmail_extra">Regards,<br/><br /></div><div class="gmail_extra">--<br />Fabrízio de Royes Mello<br
/>Consultoria/CoachingPostgreSQL<br />>> Timbira: <a
href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div></div>