object description for FDW user mappings

Started by Alvaro Herreraabout 11 years ago8 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com

When commit cae565e503 introduced FDW user mappings, it used this in
getObjectDescription for them:

appendStringInfo(&buffer, _("user mapping for %s"), usename);

This was later mostly copied (by yours truly) as object identity by
commit f8348ea32e wherein I used this:
appendStringInfo(&buffer, "%s", usename);

As it turns out, this is wrong, because the pg_user_mapping catalog has
a two-column "primary key" which is user OID and server OID. Therefore
it seems to me that the correct object description and identity must
include both username and server name. I propose we change the above to
this:

appendStringInfo(&buffer, _("user mapping for %s in server %s"), usename,
srv->servername);

I propose to apply the (appropriately rebased) attached patch to all
back branches. Note in particular the wording changes in some error
messages in the foreign_data test.

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

Attachments:

0001-fix-user-mapping-object-description-identity.patchtext/x-diff; charset=us-asciiDownload+32-23
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: object description for FDW user mappings

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

When commit cae565e503 introduced FDW user mappings, it used this in
getObjectDescription for them:
appendStringInfo(&buffer, _("user mapping for %s"), usename);

This was later mostly copied (by yours truly) as object identity by
commit f8348ea32e wherein I used this:
appendStringInfo(&buffer, "%s", usename);

As it turns out, this is wrong, because the pg_user_mapping catalog has
a two-column "primary key" which is user OID and server OID. Therefore
it seems to me that the correct object description and identity must
include both username and server name. I propose we change the above to
this:

appendStringInfo(&buffer, _("user mapping for %s in server %s"), usename,
srv->servername);

+1 for the concept, but to be nitpicky, "in" doesn't seem like the right
word here. "on server" would read better to me; or perhaps "at server".

regards, tom lane

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

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tom Lane (#2)
Re: object description for FDW user mappings

On 06-03-2015 AM 01:32, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

appendStringInfo(&buffer, _("user mapping for %s in server %s"), usename,
srv->servername);

+1 for the concept, but to be nitpicky, "in" doesn't seem like the right
word here. "on server" would read better to me; or perhaps "at server".

One more option may be "for server" (reading the doc for CREATE USER MAPPING)

Thanks,
Amit

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

#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#3)
Re: object description for FDW user mappings

On 06-03-2015 AM 09:18, Amit Langote wrote:

On 06-03-2015 AM 01:32, Tom Lane wrote:

+1 for the concept, but to be nitpicky, "in" doesn't seem like the right
word here. "on server" would read better to me; or perhaps "at server".

One more option may be "for server" (reading the doc for CREATE USER MAPPING)

Oh, I see it's been done that way already.

Thanks,
Amit

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Langote (#3)
Re: object description for FDW user mappings

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:

On 06-03-2015 AM 01:32, Tom Lane wrote:

+1 for the concept, but to be nitpicky, "in" doesn't seem like the right
word here. "on server" would read better to me; or perhaps "at server".

One more option may be "for server" (reading the doc for CREATE USER MAPPING)

Hm, but then you'd have "user mapping for foo for server bar", which
doesn't read so nicely either.

regards, tom lane

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

#6Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tom Lane (#5)
Re: object description for FDW user mappings

On 06-03-2015 AM 09:30, Tom Lane wrote:

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:

One more option may be "for server" (reading the doc for CREATE USER MAPPING)

Hm, but then you'd have "user mapping for foo for server bar", which
doesn't read so nicely either.

Yeah, I had totally missed the "for foo" part; I was thinking of it as "of foo".

By the way, in this case, is "foo" the name/id of a local user or does it
really refer to some "foo on the remote server"?

Thanks,
Amit

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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Langote (#6)
Re: object description for FDW user mappings

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:

By the way, in this case, is "foo" the name/id of a local user or does it
really refer to some "foo on the remote server"?

It's the name of a local user. I see your point that somebody might
misread this as suggesting that it's a remote username, but not sure
that there's anything great we can do here to disambiguate that.

regards, tom lane

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

#8David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#7)
Re: object description for FDW user mappings

Tom Lane-2 wrote

Amit Langote &lt;

Langote_Amit_f8@.co

&gt; writes:

By the way, in this case, is "foo" the name/id of a local user or does it
really refer to some "foo on the remote server"?

It's the name of a local user. I see your point that somebody might
misread this as suggesting that it's a remote username, but not sure
that there's anything great we can do here to disambiguate that.

Yeah, clarifying that point seems to add verbosity for little gain.

On the message aspect is it against style to use composite notation in a
case like this?

"user mapping (%s, %s)"

It is basically a single, if compound, noun so adding in/at/to doesn't feel
right...

David J.

--
View this message in context: http://postgresql.nabble.com/object-description-for-FDW-user-mappings-tp5840655p5840729.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

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