Re: [HACKERS] Foreign table permissions and cloning
2011/5/11 Shigeru Hanada han...@metrosystems.co.jp: (2011/04/26 5:42), Robert Haas wrote: OK. Turned out a little more cleanup was needed to make this all the way consistent with how we handle views; I have now done that. I noticed that some fixes would be needed for consistency about foreign table privileges. Attached patch includes fixes below: 1) psql doesn't complete FOREIGN TABLE after GRANT/REVOKE. 2) pg_dump uses GRANT .. ON TABLE for foreign tables, instead of ON FOREIGN TABLE. 3) GRANT document mentions that ALL TABLES includes foreign tables too. 4) Rows of information_schema.foreign_tables/foreign_table_options are visible to users who have any privileges on the foreign table. Thanks; I'm embarrassed I didn't notice those things myself. I've committed this patch with very slight adjustment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
(2011/04/26 5:42), Robert Haas wrote: OK. Turned out a little more cleanup was needed to make this all the way consistent with how we handle views; I have now done that. I noticed that some fixes would be needed for consistency about foreign table privileges. Attached patch includes fixes below: 1) psql doesn't complete FOREIGN TABLE after GRANT/REVOKE. 2) pg_dump uses GRANT .. ON TABLE for foreign tables, instead of ON FOREIGN TABLE. 3) GRANT document mentions that ALL TABLES includes foreign tables too. 4) Rows of information_schema.foreign_tables/foreign_table_options are visible to users who have any privileges on the foreign table. Regards, -- Shigeru Hanada diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml index 93e8332..689aba5 100644 *** a/doc/src/sgml/ref/grant.sgml --- b/doc/src/sgml/ref/grant.sgml *** GRANT replaceable class=PARAMETERrol *** 101,107 There is also an option to grant privileges on all objects of the same type within one or more schemas. This functionality is currently supported only for tables, sequences, and functions (but note that literalALL !TABLES/ is considered to include views). /para para --- 101,107 There is also an option to grant privileges on all objects of the same type within one or more schemas. This functionality is currently supported only for tables, sequences, and functions (but note that literalALL !TABLES/ is considered to include views and foreign tables). /para para diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index c623fb7..452a0ea 100644 *** a/src/backend/catalog/information_schema.sql --- b/src/backend/catalog/information_schema.sql *** CREATE VIEW _pg_foreign_tables AS *** 2557,2564 WHERE w.oid = s.srvfdw AND u.oid = c.relowner AND (pg_has_role(c.relowner, 'USAGE') !OR has_table_privilege(c.oid, 'SELECT') !OR has_any_column_privilege(c.oid, 'SELECT')) AND n.oid = c.relnamespace AND c.oid = t.ftrelid AND c.relkind = 'f' --- 2557,2564 WHERE w.oid = s.srvfdw AND u.oid = c.relowner AND (pg_has_role(c.relowner, 'USAGE') !OR has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER') !OR has_any_column_privilege(c.oid, 'SELECT, INSERT, UPDATE, REFERENCES')) AND n.oid = c.relnamespace AND c.oid = t.ftrelid AND c.relkind = 'f' diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 30366d2..e474a69 100644 *** a/src/bin/pg_dump/pg_dump.c --- b/src/bin/pg_dump/pg_dump.c *** dumpTable(Archive *fout, TableInfo *tbin *** 11804,11810 namecopy = strdup(fmtId(tbinfo-dobj.name)); dumpACL(fout, tbinfo-dobj.catId, tbinfo-dobj.dumpId, (tbinfo-relkind == RELKIND_SEQUENCE) ? SEQUENCE : - (tbinfo-relkind == RELKIND_FOREIGN_TABLE) ? FOREIGN TABLE : TABLE, namecopy, NULL, tbinfo-dobj.name, tbinfo-dobj.namespace-dobj.name, tbinfo-rolname, --- 11804,11809 diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 3ef2fa4..02f4aea 100644 *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** psql_completion(char *text, int start, i *** 2234,2240 UNION SELECT 'DATABASE' UNION SELECT 'FOREIGN DATA WRAPPER' UNION SELECT 'FOREIGN SERVER' - UNION SELECT 'FOREIGN TABLE' UNION SELECT 'FUNCTION' UNION SELECT 'LANGUAGE' UNION SELECT 'LARGE OBJECT' --- 2234,2239 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Wed, Apr 20, 2011 at 11:08 AM, Robert Haas robertmh...@gmail.com wrote:
On Wed, Apr 20, 2011 at 9:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Shigeru Hanada han...@metrosystems.co.jp writes:
Attached patch implements along specifications below. It also includes
documents and regression tests. Some of regression tests might be
redundant and removable.
1) GRANT privilege [(column_list)] ON [TABLE] TO role also work for
foreign tables as well as regular tables, if specified privilege was
SELECT. This might seem little inconsistent but I feel natural to use
this syntax for SELECT-able objects. Anyway, such usage can be disabled
with trivial fix.
It seems really seriously inconsistent to do that at the same time that
you make other forms of GRANT treat foreign tables as a separate class
of object. I think if they're going to be a separate class of object,
they should be separate, full stop. Making them just mostly separate
will confuse people no end.
I agree.
Hmm, it appears we had some pre-existing inconsistency here, because
ALL TABLES IN schema currently includes views. That's weird, but
it'll be even more weird if we adopt the approach suggested by this
patch, which creates ALL FOREIGN TABLES IN schema but allows ALL
TABLES IN schema to go on including views. Maybe there is an
argument for having ALL {TABLES|VIEWS|FOREIGN TABLES} IN schema - or
maybe there isn't - but having two out of the three of them doesn't do
anything for me. For now I think we should go with the path of least
resistance and just document that ALL TABLES IN schema now includes
not only views but also foreign tables.
Putting that together with the comments already made upthread, the
only behavior changes I think we should make here are:
- Add GRANT privilege [(column_list)] ON FOREIGN TABLE table TO role.
- Require that the argument to GRANT privilege [(column_list)] ON
TABLE TO role be an ordinary table, not a foreign table.
That looks like enough to make foreign table handling consistent with
what we're already doing.
Barring objections, I'll go make that happen.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
Robert Haas robertmh...@gmail.com writes:
Hmm, it appears we had some pre-existing inconsistency here, because
ALL TABLES IN schema currently includes views. That's weird, but
it'll be even more weird if we adopt the approach suggested by this
patch, which creates ALL FOREIGN TABLES IN schema but allows ALL
TABLES IN schema to go on including views. Maybe there is an
argument for having ALL {TABLES|VIEWS|FOREIGN TABLES} IN schema - or
maybe there isn't - but having two out of the three of them doesn't do
anything for me.
Yeah, that's a fair point. Another issue is that eventually foreign
tables will probably have some update capability, so designing GRANT
on the assumption that only SELECT should be allowed is a mistake.
In fact, I'd argue that GRANT ought not be enforcing such an assumption
even today, especially if it leads to asymmetry there. Let somebody
GRANT UPDATE if they want to --- there's no need to throw an error until
the update operation is actually tried.
Putting that together with the comments already made upthread, the
only behavior changes I think we should make here are:
- Add GRANT privilege [(column_list)] ON FOREIGN TABLE table TO role.
- Require that the argument to GRANT privilege [(column_list)] ON
TABLE TO role be an ordinary table, not a foreign table.
I think this might be going in the wrong direction given the above
thoughts. At the very least you're going to have to make sure the
prohibition is easily reversible.
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
Re: [HACKERS] Foreign table permissions and cloning
On Mon, Apr 25, 2011 at 1:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
Hmm, it appears we had some pre-existing inconsistency here, because
ALL TABLES IN schema currently includes views. That's weird, but
it'll be even more weird if we adopt the approach suggested by this
patch, which creates ALL FOREIGN TABLES IN schema but allows ALL
TABLES IN schema to go on including views. Maybe there is an
argument for having ALL {TABLES|VIEWS|FOREIGN TABLES} IN schema - or
maybe there isn't - but having two out of the three of them doesn't do
anything for me.
Yeah, that's a fair point. Another issue is that eventually foreign
tables will probably have some update capability, so designing GRANT
on the assumption that only SELECT should be allowed is a mistake.
In fact, I'd argue that GRANT ought not be enforcing such an assumption
even today, especially if it leads to asymmetry there. Let somebody
GRANT UPDATE if they want to --- there's no need to throw an error until
the update operation is actually tried.
Putting that together with the comments already made upthread, the
only behavior changes I think we should make here are:
- Add GRANT privilege [(column_list)] ON FOREIGN TABLE table TO role.
- Require that the argument to GRANT privilege [(column_list)] ON
TABLE TO role be an ordinary table, not a foreign table.
I think this might be going in the wrong direction given the above
thoughts. At the very least you're going to have to make sure the
prohibition is easily reversible.
I'm not sure I quite understood what you were saying there, but I'm
coming around to the view that this is already 100% consistent with
the way views are handled:
rhaas=# create view v as select 1;
CREATE VIEW
rhaas=# grant delete on v to bob;
GRANT
rhaas=# grant delete on table v to bob;
GRANT
If that works for a view, it also ought to work for a foreign table,
which I think is what you were saying.
So now I think this is just a documentation bug.
Do you agree?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
Robert Haas robertmh...@gmail.com writes: On Mon, Apr 25, 2011 at 1:45 PM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not sure I quite understood what you were saying there, but I'm coming around to the view that this is already 100% consistent with the way views are handled: rhaas=# create view v as select 1; CREATE VIEW rhaas=# grant delete on v to bob; GRANT rhaas=# grant delete on table v to bob; GRANT If that works for a view, it also ought to work for a foreign table, which I think is what you were saying. Yeah, the existing precedent (not only for GRANT but for some other things like ALTER TABLE) is that a command that says TABLE is allowed to apply to other relation types if it makes sense to apply it. It's only when you name some other object type that we get picky about the relkind matching exactly. This is probably more historical than anything else, but it's the precedent and we shouldn't make foreign tables be the only thing not following the precedent. So now I think this is just a documentation bug. If the code already works like that for foreign tables, then no behavioral change is needed. 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
Re: [HACKERS] Foreign table permissions and cloning
On mån, 2011-04-25 at 13:35 -0400, Robert Haas wrote:
Hmm, it appears we had some pre-existing inconsistency here, because
ALL TABLES IN schema currently includes views.
Which makes sense because you use GRANT ... ON TABLE to grant privileges
to views.
That's weird, but
it'll be even more weird if we adopt the approach suggested by this
patch, which creates ALL FOREIGN TABLES IN schema but allows ALL
TABLES IN schema to go on including views. Maybe there is an
argument for having ALL {TABLES|VIEWS|FOREIGN TABLES} IN schema - or
maybe there isn't - but having two out of the three of them doesn't do
anything for me. For now I think we should go with the path of least
resistance and just document that ALL TABLES IN schema now includes
not only views but also foreign tables.
Yes.
Putting that together with the comments already made upthread, the
only behavior changes I think we should make here are:
- Add GRANT privilege [(column_list)] ON FOREIGN TABLE table TO role.
- Require that the argument to GRANT privilege [(column_list)] ON
TABLE TO role be an ordinary table, not a foreign table.
But that would be contrary to the SQL standard. The current behavior is
fine, AFAICT.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Mon, Apr 25, 2011 at 2:02 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
On Mon, Apr 25, 2011 at 1:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
I'm not sure I quite understood what you were saying there, but I'm
coming around to the view that this is already 100% consistent with
the way views are handled:
rhaas=# create view v as select 1;
CREATE VIEW
rhaas=# grant delete on v to bob;
GRANT
rhaas=# grant delete on table v to bob;
GRANT
If that works for a view, it also ought to work for a foreign table,
which I think is what you were saying.
Yeah, the existing precedent (not only for GRANT but for some other
things like ALTER TABLE) is that a command that says TABLE is allowed
to apply to other relation types if it makes sense to apply it. It's
only when you name some other object type that we get picky about the
relkind matching exactly. This is probably more historical than
anything else, but it's the precedent and we shouldn't make foreign
tables be the only thing not following the precedent.
So now I think this is just a documentation bug.
If the code already works like that for foreign tables, then no
behavioral change is needed.
OK, let's test that:
rhaas=# create foreign data wrapper dummy;
CREATE FOREIGN DATA WRAPPER
rhaas=# create server s1 foreign data wrapper dummy;
CREATE SERVER
rhaas=# create foreign table ft (a int) server s1;
CREATE FOREIGN TABLE
rhaas=# grant delete on ft to bob;
ERROR: foreign table ft only supports SELECT privileges
rhaas=# grant delete on table ft to bob;
ERROR: foreign table ft only supports SELECT privileges
So, nope, not the same. Also for comparison:
rhaas=# create sequence blarg;
CREATE SEQUENCE
rhaas=# grant delete on blarg to bob;
WARNING: sequence blarg only supports USAGE, SELECT, and UPDATE privileges
GRANT
This appears to be because ExecGrant_Relation() has this:
else if (pg_class_tuple-relkind == RELKIND_FOREIGN_TABLE)
{
if (this_privileges ~((AclMode) ACL_ALL_RIGHTS_FOREIGN_TABLE))
{
ereport(ERROR,
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
errmsg(foreign table \%s\ only
supports SELECT privileges,
NameStr(pg_class_tuple-relname;
}
}
There's a similar stanza for sequences, but that one uses
ereport(WARNING...) rather than ereport(ERROR...). We could either
remove that stanza entirely (making foreign tables consistent with
views) or change ERROR to WARNING (making it consistent with
sequences).
If we remove it entirely, then we'll presumably also want to remove
this chunk further down:
else if (pg_class_tuple-relkind == RELKIND_FOREIGN_TABLE
this_privileges ~((AclMode) ACL_SELECT))
{
/* Foreign tables have the same restriction as sequences. */
ereport(WARNING,
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
errmsg(foreign table \%s\ only supports
SELECT column privileges,
NameStr(pg_class_tuple-relname;
this_privileges = (AclMode) ACL_SELECT;
}
Thoughts?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
Robert Haas robertmh...@gmail.com writes: ... There's a similar stanza for sequences, but that one uses ereport(WARNING...) rather than ereport(ERROR...). We could either remove that stanza entirely (making foreign tables consistent with views) or change ERROR to WARNING (making it consistent with sequences). Well, the relevant point here is that there's little or no likelihood that we'll ever care to support direct UPDATE on sequences. This is exactly not the case for foreign tables. So I would argue that GRANT should handle them like views; certainly not be even more strict than it is for sequences. IOW, yeah, let's drop these two checks. 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
Re: [HACKERS] Foreign table permissions and cloning
On Mon, Apr 25, 2011 at 7:02 PM, Tom Lane t...@sss.pgh.pa.us wrote: Yeah, the existing precedent (not only for GRANT but for some other things like ALTER TABLE) is that a command that says TABLE is allowed to apply to other relation types if it makes sense to apply it. It's only when you name some other object type that we get picky about the relkind matching exactly. This is probably more historical than anything else, but it's the precedent and we shouldn't make foreign tables be the only thing not following the precedent. Actually I vaguely remember some earlier pass through this code to make it more consistent. IIRC we previously had some commands that could only be done through ALTER TABLE even for things like sequences and views and other commands that had corresponding ALTER VIEW commands. I'll try to see if I can dig up the messages on the topic. -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Mon, Apr 25, 2011 at 2:31 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... There's a similar stanza for sequences, but that one uses ereport(WARNING...) rather than ereport(ERROR...). We could either remove that stanza entirely (making foreign tables consistent with views) or change ERROR to WARNING (making it consistent with sequences). Well, the relevant point here is that there's little or no likelihood that we'll ever care to support direct UPDATE on sequences. This is exactly not the case for foreign tables. So I would argue that GRANT should handle them like views; certainly not be even more strict than it is for sequences. IOW, yeah, let's drop these two checks. OK. Turned out a little more cleanup was needed to make this all the way consistent with how we handle views; I have now done that. pauses to listen for screaming noises -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
(2011/04/15 3:43), Robert Haas wrote:
On Fri, Apr 1, 2011 at 1:29 AM, Shigeru HANADA
han...@metrosystems.co.jp wrote:
In addition to the 2nd GRANT above, GRANT SELECT (colour) ON stuff TO
user_a (omitting TABLE) will succeed too because parser assumes that
the target object is a regular table if object type was TABLE or
omitted. This inconsistent behavior would be an oversight and need to
be fixed.
+1.
How about to drop GRANT xxx ON FOREIGN TABLE foo syntax support and
use GRANT xxx ON [TABLE] foo for foreign tables? ISTM that ON
FOREIGN TABLE specification is useless because possible privilege
type would be same as TABLE.
-1. We should be consistent about treating foreign tables as their
own object type - and the possible privilege types are NOT the same -
only SELECT is supported.
Probabry we should mention in GRANT documents that ALL TABLES
IN SCHEMA is considered to include foreign tables.
Or else change the behavior so that it doesn't, which would probably be my vote.
Thanks for the comments. I agree that foreign table is a different
object type from regular table or view in the context of ACL.
Attached patch implements along specifications below. It also includes
documents and regression tests. Some of regression tests might be
redundant and removable.
1) GRANT privilege [(column_list)] ON [TABLE] TO role also work for
foreign tables as well as regular tables, if specified privilege was
SELECT. This might seem little inconsistent but I feel natural to use
this syntax for SELECT-able objects. Anyway, such usage can be disabled
with trivial fix.
2) GRANT privilege [(column_list)] ON FOREIGN TABLE table TO role
works only for foreign tables, and accepts only SELECT as privilege.
3) GRANT privilege ON ALL TABLES IN SCHEMA schema TO role doesn't
affect any foreign table in the schema.
4) GRANT privilege [(column_list)] ON ALL FOREIGN TABLES IN SCHEMA
schema TO role works for all foreign tables in the schema, but not
affect any regular table, view or sequence in the schema.
BTW, I noticed some issues about ACL below. Some of them might have to
be fixed in future.
a) GRANT privilege(column_list) ON ALL TABLES IN SCHEMA schema works
fine if all of the tables in the schema have all of listed columns. It
is not documented, and might be unintentional.
b) ALTER DEFAULT PRIVILEGE doesn't support foreign tables.
c) Currently SELECT privilege allow user to retrieve contents of the
foreign table, but this is different from SQL/MED Standard. Should this
difference be mentioned in GRANT document?
[4.14.1 Privileges]
NOTE 9 — Privileges granted on foreign tables are not privileges to
use the data constituting foreign tables, but privileges to use the
definitions of the foreign tables. The privileges to access the data
constituting the foreign tables are enforced by the foreign server,
based on the user mapping. Consequently, a request by an SQL-client
to access external data may raise exceptions.
Regards,
--
Shigeru Hanada
diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 72ecc45..3ad9fab 100644
*** a/doc/src/sgml/ref/grant.sgml
--- b/doc/src/sgml/ref/grant.sgml
*** GRANT { { SELECT | INSERT | UPDATE | REF
*** 32,37
--- 32,46
ON [ TABLE ] replaceable class=PARAMETERtable_name/replaceable [,
...]
TO { [ GROUP ] replaceable class=PARAMETERrole_name/replaceable |
PUBLIC } [, ...] [ WITH GRANT OPTION ]
+ GRANT { SELECT | ALL [ PRIVILEGES ] }
+ ON { FOREIGN TABLE replaceable
class=PARAMETERforeign_table_name/replaceable [, ...]
+ | ALL FOREIGN TABLES IN SCHEMA replaceable
class=PARAMETERschema_name/replaceable [, ...] }
+ TO { [ GROUP ] replaceable class=PARAMETERrole_name/replaceable |
PUBLIC } [, ...] [ WITH GRANT OPTION ]
+
+ GRANT { SELECT | ALL [ PRIVILEGES ] } ( replaceable
class=PARAMETERcolumn/replaceable [, ...] )
+ ON FOREIGN TABLE replaceable
class=PARAMETERforeign_table_name/replaceable [, ...]
+ TO { [ GROUP ] replaceable class=PARAMETERrole_name/replaceable |
PUBLIC } [, ...] [ WITH GRANT OPTION ]
+
GRANT { { USAGE | SELECT | UPDATE }
[, ...] | ALL [ PRIVILEGES ] }
ON { SEQUENCE replaceable class=PARAMETERsequence_name/replaceable
[, ...]
*** GRANT replaceable class=PARAMETERrol
*** 81,87
para
The commandGRANT/command command has two basic variants: one
that grants privileges on a database object (table, column, view, sequence,
!database, foreign-data wrapper, foreign server, function,
procedural language, schema, or tablespace), and one that grants
membership in a role. These variants are similar in many ways, but
they are different enough to be described separately.
--- 90,96
para
The commandGRANT/command command has two basic variants: one
that grants privileges on a database object (table, column, view, sequence,
!foreign table, database, foreign-data wrapper,
Re: [HACKERS] Foreign table permissions and cloning
Shigeru Hanada han...@metrosystems.co.jp writes: Attached patch implements along specifications below. It also includes documents and regression tests. Some of regression tests might be redundant and removable. 1) GRANT privilege [(column_list)] ON [TABLE] TO role also work for foreign tables as well as regular tables, if specified privilege was SELECT. This might seem little inconsistent but I feel natural to use this syntax for SELECT-able objects. Anyway, such usage can be disabled with trivial fix. It seems really seriously inconsistent to do that at the same time that you make other forms of GRANT treat foreign tables as a separate class of object. I think if they're going to be a separate class of object, they should be separate, full stop. Making them just mostly separate will confuse people no end. 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
Re: [HACKERS] Foreign table permissions and cloning
On Wed, Apr 20, 2011 at 9:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Shigeru Hanada han...@metrosystems.co.jp writes: Attached patch implements along specifications below. It also includes documents and regression tests. Some of regression tests might be redundant and removable. 1) GRANT privilege [(column_list)] ON [TABLE] TO role also work for foreign tables as well as regular tables, if specified privilege was SELECT. This might seem little inconsistent but I feel natural to use this syntax for SELECT-able objects. Anyway, such usage can be disabled with trivial fix. It seems really seriously inconsistent to do that at the same time that you make other forms of GRANT treat foreign tables as a separate class of object. I think if they're going to be a separate class of object, they should be separate, full stop. Making them just mostly separate will confuse people no end. I agree. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On 15 April 2011 04:26, Tom Lane t...@sss.pgh.pa.us wrote: Why is this a documentation issue and not a code issue? IMO we should flat out reject both NOT NULL and DEFAULT declarations on foreign tables, until such time as we're prepared to do something useful with them. Reasons: If the removal the redundant declarations is do-able for this release, that would be preferable. And if that change is to happen, I guess it has to start happening immediately, letting the pgAdmin guys know too. 1. Accepting non-functional constraint declarations is something we've been heard to ridicule mysql for. With good reason. Fair point. 2. It probably won't be too long before the planner makes optimization decisions that assume NOT NULL declarations to be truthful. When that day comes, I don't want to be seeing an exception for foreign tables in that logic. Makes sense. 3. When we do get around to making it actually work, we will have a backwards-compatibility problem if prior versions accepted the declaration but treated it as a no-op. Probably the most important point. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Thu, Apr 14, 2011 at 8:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: 1. Accepting non-functional constraint declarations is something we've been heard to ridicule mysql for. With good reason. 2. It probably won't be too long before the planner makes optimization decisions that assume NOT NULL declarations to be truthful. When that day comes, I don't want to be seeing an exception for foreign tables in that logic. 3. When we do get around to making it actually work, we will have a backwards-compatibility problem if prior versions accepted the declaration but treated it as a no-op. The planner *already* makes optimization decisions that assume NOT NULL declarations are truthful (see: left join reordering). That's why we have them for foreign tables, and why we eventually will need constraints as well. Of course, we have no guarantee that the data on the foreign side matches those constraints. But we don't have any guarantee that it matches the data type declarations, either. We could insist that every column must be of type text and allow NULLs, but that seems like it would be losing rather a lot of the functionality. The point of a datatype declaration, or a NOT NULL declaration, or any other such thing with respect to foreign tables is that the user is making an assertion about what the data looks like, hopefully with some cooperation from the FDW. It's ridiculous to suppose that we will really be able to control anything at all about the data on the foreign side, even the number of columns. But our job is to shove whatever information is present into the schema the user has specified, or throw an error. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Fri, Apr 1, 2011 at 5:13 AM, Thom Brown t...@linux.com wrote: On 1 April 2011 12:57, Shigeru HANADA han...@metrosystems.co.jp wrote: NOT NULL constraint on foreign table is just declaration and can't force data integrity. And I noticed that CREATE FOREIGN TABLE document doesn't mention that serial and bigserial can't be used in foreign table. Please see foreign_table_doc.patch for this fix. I'd be inclined to generalise it to say that default values can't be used on a foreign table, and then say that as a result, serial and bigserial can't be used. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Fri, Apr 1, 2011 at 1:29 AM, Shigeru HANADA han...@metrosystems.co.jp wrote: In addition to the 2nd GRANT above, GRANT SELECT (colour) ON stuff TO user_a (omitting TABLE) will succeed too because parser assumes that the target object is a regular table if object type was TABLE or omitted. This inconsistent behavior would be an oversight and need to be fixed. +1. How about to drop GRANT xxx ON FOREIGN TABLE foo syntax support and use GRANT xxx ON [TABLE] foo for foreign tables? ISTM that ON FOREIGN TABLE specification is useless because possible privilege type would be same as TABLE. -1. We should be consistent about treating foreign tables as their own object type - and the possible privilege types are NOT the same - only SELECT is supported. Probabry we should mention in GRANT documents that ALL TABLES IN SCHEMA is considered to include foreign tables. Or else change the behavior so that it doesn't, which would probably be my vote. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
Robert Haas robertmh...@gmail.com writes: On Fri, Apr 1, 2011 at 5:13 AM, Thom Brown t...@linux.com wrote: On 1 April 2011 12:57, Shigeru HANADA han...@metrosystems.co.jp wrote: NOT NULL constraint on foreign table is just declaration and can't force data integrity. And I noticed that CREATE FOREIGN TABLE document doesn't mention that serial and bigserial can't be used in foreign table. Please see foreign_table_doc.patch for this fix. I'd be inclined to generalise it to say that default values can't be used on a foreign table, and then say that as a result, serial and bigserial can't be used. +1. Why is this a documentation issue and not a code issue? IMO we should flat out reject both NOT NULL and DEFAULT declarations on foreign tables, until such time as we're prepared to do something useful with them. Reasons: 1. Accepting non-functional constraint declarations is something we've been heard to ridicule mysql for. With good reason. 2. It probably won't be too long before the planner makes optimization decisions that assume NOT NULL declarations to be truthful. When that day comes, I don't want to be seeing an exception for foreign tables in that logic. 3. When we do get around to making it actually work, we will have a backwards-compatibility problem if prior versions accepted the declaration but treated it as a no-op. 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
Re: [HACKERS] Foreign table permissions and cloning
On Fri, 1 Apr 2011 00:54:18 +0100 Thom Brown t...@linux.com wrote: I've noticed some weirdness when trying to grant various types of permissions on a foreign table and thought I'd report it here: postgres=# \d stuff Foreign table public.stuff Column | Type | Modifiers +-+--- id | integer | colour | text| animal | text| Server: file postgres=# GRANT SELECT (colour) ON FOREIGN TABLE stuff TO user_a; ERROR: column privileges are only valid for relations postgres=# GRANT SELECT (colour) ON TABLE stuff TO user_a; GRANT postgres=# GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_a; ERROR: syntax error at or near FOREIGN LINE 1: GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_... ^ Granting select for all tables in a schema to a user will affect foreign tables however. And column-level permissions work with foreign tables if you refer to them as regular tables in the GRANT/REVOKE statement. Using the term FOREIGN TABLE in a GRANT statement isn't documented. I suspect this will need its own entry in the syntax definition section of the GRANT and REVOKE reference pages. In addition to the 2nd GRANT above, GRANT SELECT (colour) ON stuff TO user_a (omitting TABLE) will succeed too because parser assumes that the target object is a regular table if object type was TABLE or omitted. This inconsistent behavior would be an oversight and need to be fixed. How about to drop GRANT xxx ON FOREIGN TABLE foo syntax support and use GRANT xxx ON [TABLE] foo for foreign tables? ISTM that ON FOREIGN TABLE specification is useless because possible privilege type would be same as TABLE. In this approach, FOREIGN TABLE (object type) is removed from privilege_target of gram.y. With this change, parser can't determine actual object type (ACL_OBJECT_RELATION or ACL_OBJECT_FOREIGN_TABLE), but it wouldn't be problem because object type will be retrieved in ExecGrant_Relation() for validate privilege type. Probabry we should mention in GRANT documents that ALL TABLES IN SCHEMA is considered to include foreign tables. Attached patch includes removing GRANT ON FOREIGN TABLE syntax fix, tab-completion fix, GRANT documents fix and additional regression tests. Regards, -- Shigeru Hanada 20110401_column_privs.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On Fri, 1 Apr 2011 01:24:20 +0100 Thom Brown t...@linux.com wrote: Also, there probably needs to be some elaboration of how a NOT NULL declaration operates on a foreign table column on the CREATE FOREIGN TABLE reference page. From what I can see, if the foreign table cannot be modified such as those defined using the file_fdw handler, it bears no relevance, and if the foreign table can be written to, it won't prevent NULL values being returned if they're already in there, just prevent them being entered (presumably). It also won't validate data in the writable foreign table upon its creation. NOT NULL constraint on foreign table is just declaration and can't force data integrity. And I noticed that CREATE FOREIGN TABLE document doesn't mention that serial and bigserial can't be used in foreign table. Please see foreign_table_doc.patch for this fix. For constraint on foreign tables, once query-time-constraint was considered, but such overhead would not be ignorable. And another problem I've found is if you try to create a table named the same as a foreign table, and you use the IF NOT EXISTS clause: postgres=# CREATE TABLE IF NOT EXISTS animals (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence animals_id_seq1 for serial column animals.id NOTICE: relation animals already exists, skipping CREATE TABLE postgres=# CREATE TABLE IF NOT EXISTS stuff (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence stuff_id_seq for serial column stuff.id NOTICE: relation stuff already exists, skipping ERROR: referenced relation stuff is not a table The reverse doesn't error though, where you attempt to create a foreign table named the same as a regular table using IF NOT EXISTS. Using int instead of serial or omitting if not exists prevends the error, so I researched root cause. CREATE TABLE with serial column is transformed into 3 DDLs: (1) CREATE SEQUENCE, for serial column (2) CREATE TABLE, skipped if table exists with same name (3) ALTER SEQUENCE OWNED BY, associate sequence with table This error occurs in (3) because process_owned_by() misunderstand that existing table is new owner, but it's a foreign server and shouldn't be used as owner. So same error occurs if the existing relation was an index or a sequence. Regards, -- Shigeru Hanada 20110401_foreign_table_doc.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On 1 April 2011 12:57, Shigeru HANADA han...@metrosystems.co.jp wrote: NOT NULL constraint on foreign table is just declaration and can't force data integrity. And I noticed that CREATE FOREIGN TABLE document doesn't mention that serial and bigserial can't be used in foreign table. Please see foreign_table_doc.patch for this fix. I'd be inclined to generalise it to say that default values can't be used on a foreign table, and then say that as a result, serial and bigserial can't be used. Using int instead of serial or omitting if not exists prevends the error, so I researched root cause. CREATE TABLE with serial column is transformed into 3 DDLs: (1) CREATE SEQUENCE, for serial column (2) CREATE TABLE, skipped if table exists with same name (3) ALTER SEQUENCE OWNED BY, associate sequence with table This error occurs in (3) because process_owned_by() misunderstand that existing table is new owner, but it's a foreign server and shouldn't be used as owner. So same error occurs if the existing relation was an index or a sequence. I see what you mean, so the error is unrelated to any foreign table support and applies to any database object that's not a regular table. Do we still want this behaviour for foreign tables, or should they be made an exception as they are a type of table? Although to be fair, I can't see the use case for it. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Foreign table permissions and cloning
Hi, I've noticed some weirdness when trying to grant various types of permissions on a foreign table and thought I'd report it here: postgres=# \d stuff Foreign table public.stuff Column | Type | Modifiers +-+--- id | integer | colour | text| animal | text| Server: file postgres=# GRANT SELECT (colour) ON FOREIGN TABLE stuff TO user_a; ERROR: column privileges are only valid for relations postgres=# GRANT SELECT (colour) ON TABLE stuff TO user_a; GRANT postgres=# GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_a; ERROR: syntax error at or near FOREIGN LINE 1: GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_... ^ Granting select for all tables in a schema to a user will affect foreign tables however. And column-level permissions work with foreign tables if you refer to them as regular tables in the GRANT/REVOKE statement. Using the term FOREIGN TABLE in a GRANT statement isn't documented. I suspect this will need its own entry in the syntax definition section of the GRANT and REVOKE reference pages. I also noticed this doesn't work: postgres=# CREATE TABLE animals (LIKE stuff); ERROR: inherited relation stuff is not a table Since LIKE doesn't maintain any sort of link with the table like INHERITS does, it would be nice if this could work in future. Thanks -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Foreign table permissions and cloning
On 1 April 2011 00:54, Thom Brown t...@linux.com wrote: Hi, I've noticed some weirdness when trying to grant various types of permissions on a foreign table and thought I'd report it here: postgres=# \d stuff Foreign table public.stuff Column | Type | Modifiers +-+--- id | integer | colour | text | animal | text | Server: file postgres=# GRANT SELECT (colour) ON FOREIGN TABLE stuff TO user_a; ERROR: column privileges are only valid for relations postgres=# GRANT SELECT (colour) ON TABLE stuff TO user_a; GRANT postgres=# GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_a; ERROR: syntax error at or near FOREIGN LINE 1: GRANT SELECT ON ALL FOREIGN TABLES IN SCHEMA public TO user_... ^ Granting select for all tables in a schema to a user will affect foreign tables however. And column-level permissions work with foreign tables if you refer to them as regular tables in the GRANT/REVOKE statement. Using the term FOREIGN TABLE in a GRANT statement isn't documented. I suspect this will need its own entry in the syntax definition section of the GRANT and REVOKE reference pages. I also noticed this doesn't work: postgres=# CREATE TABLE animals (LIKE stuff); ERROR: inherited relation stuff is not a table Since LIKE doesn't maintain any sort of link with the table like INHERITS does, it would be nice if this could work in future. Also, there probably needs to be some elaboration of how a NOT NULL declaration operates on a foreign table column on the CREATE FOREIGN TABLE reference page. From what I can see, if the foreign table cannot be modified such as those defined using the file_fdw handler, it bears no relevance, and if the foreign table can be written to, it won't prevent NULL values being returned if they're already in there, just prevent them being entered (presumably). It also won't validate data in the writable foreign table upon its creation. And another problem I've found is if you try to create a table named the same as a foreign table, and you use the IF NOT EXISTS clause: postgres=# CREATE TABLE IF NOT EXISTS animals (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence animals_id_seq1 for serial column animals.id NOTICE: relation animals already exists, skipping CREATE TABLE postgres=# CREATE TABLE IF NOT EXISTS stuff (id serial, stuff text); NOTICE: CREATE TABLE will create implicit sequence stuff_id_seq for serial column stuff.id NOTICE: relation stuff already exists, skipping ERROR: referenced relation stuff is not a table The reverse doesn't error though, where you attempt to create a foreign table named the same as a regular table using IF NOT EXISTS. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
