Обсуждение: how to find out whether a view is updatable
I was looking for a way in which the average psql user could learn whether a view is updatable. I was expecting something in \d, \d+, \dv, \dv+, or a NOTICE from CREATE VIEW. So far, the only way appears to be through the information schema or the underlying pg_view_is_updatable function. Not even pg_views shows anything. Is this intentional or an oversight?
On Wed, Jun 5, 2013 at 12:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
-- I was looking for a way in which the average psql user could learn
whether a view is updatable. I was expecting something in \d, \d+, \dv,
\dv+, or a NOTICE from CREATE VIEW. So far, the only way appears to be
through the information schema or the underlying pg_view_is_updatable
function. Not even pg_views shows anything. Is this intentional or an
oversight?
Just by recalling the thread, an oversight. Having this information in ¥dv+ would
be indeed a nice addition.
Michael
On 4 June 2013 23:35, Michael Paquier <michael.paquier@gmail.com> wrote: > > > > On Wed, Jun 5, 2013 at 12:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote: >> >> I was looking for a way in which the average psql user could learn >> whether a view is updatable. I was expecting something in \d, \d+, \dv, >> \dv+, or a NOTICE from CREATE VIEW. So far, the only way appears to be >> through the information schema or the underlying pg_view_is_updatable >> function. Not even pg_views shows anything. Is this intentional or an >> oversight? > > Just by recalling the thread, an oversight. Having this information in ¥dv+ > would > be indeed a nice addition. Yes, agreed -- something like this would be nice. It's not just views though -- foreign tables may now also be updatable, so I think it should work for \d+ in general, not just \dv+. Perhaps we should add a new column to \d+'s list of relations (provided that doesn't make it too wide) and add an extra line at the end of the \d+ description for a single relation. Should this also distinguish between insertable, updatable and deletable (i.e., support for INSERT, UPDATE and DELETE)? I'm still not happy with pg_view_is_updatable() et al. and the information_schema views. I accept that the information_schema views have to be the way they are because that's what's defined in the standard, but as it stands, the distinction between updatable and trigger-updatable makes it impossible in general to answer the simple question "does foo support UPDATEs?". I'm thinking what we really need is a single function with a slightly different signature, that can be used to support both the information schema views and psql's \d+ (and potentially other client apps). Perhaps something like:- pg_relation_is_updatable(include_triggers boolean) returns int which would work for all relation kinds, returning a bitmask indicating which of the operations (INSERT, UPDATE and DELETE) are supported, together with a matching function in the FDW API. Thoughts? Dean
On 5 June 2013 08:59, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > I'm still not happy with pg_view_is_updatable() et al. and the > information_schema views. I accept that the information_schema views > have to be the way they are because that's what's defined in the > standard, but as it stands, the distinction between updatable and > trigger-updatable makes it impossible in general to answer the simple > question "does foo support UPDATEs?". > > I'm thinking what we really need is a single function with a slightly > different signature, that can be used to support both the information > schema views and psql's \d+ (and potentially other client apps). > Perhaps something like:- > > pg_relation_is_updatable(include_triggers boolean) > returns int > OK, here's what it looks like using this approach: FUNCTION pg_relation_is_updatable(reloid oid, include_triggers boolean) RETURNS integer FUNCTION pg_column_is_updatable(reloid oid, attnum integer, include_triggers boolean) RETURNS boolean These replace pg_view_is_updatable() and pg_view_is_insertable(). I think I definitely prefer this over the old API, because it gives much greater flexibility. The information schema views all pass include_triggers = false for compatibility with the standard. The return value from pg_relation_is_updatable() is now an integer bitmask reflecting whether or not the relation is insertable, updatable and/or deletable. psql and other clients can more usefully pass include_triggers = true to determine whether a relation actually supports INSERT, UPDATE and DELETE, including checks for INSTEAD OF triggers on the specified relation or any underlying base relations. I thought about having pg_relation_is_updatable() return text, like the GRANT support functions, but I thought that it would make the information schema views harder to write, using a single call to check for updatable+deletable, whereas integer bit operations are easy. There is a backwards-incompatible change to the information schema, reflected in the regression tests: if a view is updatable but not deletable, the relevant rows in information_schema.columns now say 'YES' --- the columns are updatable, even though the relation as a whole isn't. I've initially defined matching FDW callback functions: int IsForeignRelUpdatable (Oid foreigntableid, bool include_triggers); bool IsForeignColUpdatable (Oid foreigntableid, int attnum, bool include_triggers); but I'm now having second thoughts about whether we should bother passing include_triggers to the FDW. If we regard the foreign table as a black box, we only care about whether it is updatable, not *how* that update is performed. Regards, Dean
Вложения
On 6 June 2013 08:09, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On 5 June 2013 08:59, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: >> I'm still not happy with pg_view_is_updatable() et al. and the >> information_schema views. I accept that the information_schema views >> have to be the way they are because that's what's defined in the >> standard, but as it stands, the distinction between updatable and >> trigger-updatable makes it impossible in general to answer the simple >> question "does foo support UPDATEs?". >> >> I'm thinking what we really need is a single function with a slightly >> different signature, that can be used to support both the information >> schema views and psql's \d+ (and potentially other client apps). >> Perhaps something like:- >> >> pg_relation_is_updatable(include_triggers boolean) >> returns int >> > > OK, here's what it looks like using this approach: > > > FUNCTION pg_relation_is_updatable(reloid oid, > include_triggers boolean) > RETURNS integer > > > FUNCTION pg_column_is_updatable(reloid oid, > attnum integer, > include_triggers boolean) > RETURNS boolean > > > These replace pg_view_is_updatable() and pg_view_is_insertable(). I > think I definitely prefer this over the old API, because it gives much > greater flexibility. > > The information schema views all pass include_triggers = false for > compatibility with the standard. The return value from > pg_relation_is_updatable() is now an integer bitmask reflecting > whether or not the relation is insertable, updatable and/or deletable. > > psql and other clients can more usefully pass include_triggers = true > to determine whether a relation actually supports INSERT, UPDATE and > DELETE, including checks for INSTEAD OF triggers on the specified > relation or any underlying base relations. > > I thought about having pg_relation_is_updatable() return text, like > the GRANT support functions, but I thought that it would make the > information schema views harder to write, using a single call to check > for updatable+deletable, whereas integer bit operations are easy. > > There is a backwards-incompatible change to the information schema, > reflected in the regression tests: if a view is updatable but not > deletable, the relevant rows in information_schema.columns now say > 'YES' --- the columns are updatable, even though the relation as a > whole isn't. > > I've initially defined matching FDW callback functions: > > > int > IsForeignRelUpdatable (Oid foreigntableid, > bool include_triggers); > > > bool > IsForeignColUpdatable (Oid foreigntableid, > int attnum, > bool include_triggers); > > > but I'm now having second thoughts about whether we should bother > passing include_triggers to the FDW. If we regard the foreign table as > a black box, we only care about whether it is updatable, not *how* > that update is performed. > Here's a more complete patch along those lines. It defines the following pair of functions to test for updatability from SQL: FUNCTION pg_catalog.pg_relation_is_updatable(reloid oid, include_triggers boolean) RETURNS integer FUNCTION pg_catalog.pg_column_is_updatable(reloid oid, attnum smallint, include_triggers boolean) RETURNS boolean and the following FDW functions: int IsForeignRelUpdatable (Oid foreigntableid); bool IsForeignColUpdatable (Oid foreigntableid, AttrNumber attnum); As an initial implementation of this API in the postgres-fdw, I've added a new option "updatable" (true by default), which can be specified as a server option or as a per-table option, to give user control over whether individual foreign tables are read-only or updatable. I called it updatable rather than "writable" or "read-only" because it might perhaps be extended in the future with separate options for "insertable" and "deletable". It could also be extended to give column-level control over updatability, or something like "use_remote_updatability" could be added, but that all feels like 9.4 material. Regards, Dean
Вложения
Sorry for my late reply.
Michael
On Sun, Jun 9, 2013 at 6:45 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
-- I called it updatable rather than "writable" or "read-only" because it
might perhaps be extended in the future with separate options for
"insertable" and "deletable". It could also be extended to give
column-level control over updatability, or something like
"use_remote_updatability" could be added, but that all feels like 9.4
material.
Yes this is definitely material for 9.4. You should add this patch to the 1st commit fest. I'll add myself as a reviewer.
Thanks,
Michael
On 11 June 2013 01:03, Michael Paquier <michael.paquier@gmail.com> wrote: > Sorry for my late reply. > > On Sun, Jun 9, 2013 at 6:45 PM, Dean Rasheed <dean.a.rasheed@gmail.com> > wrote: >> >> I called it updatable rather than "writable" or "read-only" because it >> might perhaps be extended in the future with separate options for >> "insertable" and "deletable". It could also be extended to give >> column-level control over updatability, or something like >> "use_remote_updatability" could be added, but that all feels like 9.4 >> material. > > > Yes this is definitely material for 9.4. You should add this patch to the > 1st commit fest. I'll add myself as a reviewer. > Thanks, > Thanks. Arguably though, the API changes are something that should be sorted out in 9.3, but I'm not sure how much of an appetite there is for that, or whether it's too late. pg_view_is_updatable() and pg_view_is_insertable() are both new to 9.3. They were designed purely to support the information schema views, but are inadequate for most other practical purposes. Once 9.3 is out, we'll be stuck with them - although of course that doesn't stop us adding more functions, I think it would be better to replace them now. Likewise the writable FDW API is new to 9.3, so I think 9.3 should at least decide on the API for a FDW to specify whether a foreign table is updatable. Regards, Dean
Dean Rasheed <dean.a.rasheed@gmail.com> writes: > On 11 June 2013 01:03, Michael Paquier <michael.paquier@gmail.com> wrote: >> Yes this is definitely material for 9.4. You should add this patch to the > Thanks. Arguably though, the API changes are something that should be > sorted out in 9.3, I agree --- I'm planning to look at this in the next few days. regards, tom lane
On Tue, Jun 11, 2013 at 4:07 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
--
Michael
sorted out in 9.3, but I'm not sure how much of an appetite there isThanks. Arguably though, the API changes are something that should be
for that, or whether it's too late.
I see, OK for the API changes on the functions, but I am not sure it is time to add new options in postgres_fdw as you do in your second patch. Unfortunately I will not be able to have a look in details at your patch, I am sick...
Michael
Dean Rasheed <dean.a.rasheed@gmail.com> writes: > Here's a more complete patch along those lines. It defines the > following pair of functions to test for updatability from SQL: > FUNCTION pg_catalog.pg_relation_is_updatable(reloid oid, > include_triggers boolean) > RETURNS integer > FUNCTION pg_catalog.pg_column_is_updatable(reloid oid, > attnum smallint, > include_triggers boolean) > RETURNS boolean > and the following FDW functions: > int IsForeignRelUpdatable (Oid foreigntableid); > bool IsForeignColUpdatable (Oid foreigntableid, > AttrNumber attnum); I'm looking at this patch now. I do not see the point of pg_column_is_updatable nor IsForeignColUpdatable --- that's loading additional complexity onto every FDW, to support what use-cases exactly? Is it really likely that (a) there are any cases out there where FDWs would support updating only some columns, and (b) anybody would care whether or not information_schema.columns reflects such a restriction accurately? So I'm inclined to drop that part. > As an initial implementation of this API in the postgres-fdw, I've > added a new option "updatable" (true by default), which can be > specified as a server option or as a per-table option, to give user > control over whether individual foreign tables are read-only or > updatable. Do we really want that as a server option? Why? regards, tom lane
On 11 June 2013 22:53, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Dean Rasheed <dean.a.rasheed@gmail.com> writes: >> Here's a more complete patch along those lines. It defines the >> following pair of functions to test for updatability from SQL: > >> FUNCTION pg_catalog.pg_relation_is_updatable(reloid oid, >> include_triggers boolean) >> RETURNS integer > >> FUNCTION pg_catalog.pg_column_is_updatable(reloid oid, >> attnum smallint, >> include_triggers boolean) >> RETURNS boolean > >> and the following FDW functions: > >> int IsForeignRelUpdatable (Oid foreigntableid); > >> bool IsForeignColUpdatable (Oid foreigntableid, >> AttrNumber attnum); > > I'm looking at this patch now. I do not see the point of > pg_column_is_updatable nor IsForeignColUpdatable --- that's loading > additional complexity onto every FDW, to support what use-cases exactly? > Is it really likely that (a) there are any cases out there where FDWs > would support updating only some columns, and (b) anybody would care > whether or not information_schema.columns reflects such a restriction > accurately? So I'm inclined to drop that part. > I originally thought of adding pg_column_is_updatable() because I was imagining supporting more of the SQL standard on updatable views, which allows for a subset of the columns to be updatable, but we could always add such a function when/if we implement that feature. As for IsForeignColUpdatable(), I think you're probably right. If it's only purpose is to support information_schema.columns, it's probably of very limited interest to anyone. >> As an initial implementation of this API in the postgres-fdw, I've >> added a new option "updatable" (true by default), which can be >> specified as a server option or as a per-table option, to give user >> control over whether individual foreign tables are read-only or >> updatable. > > Do we really want that as a server option? Why? > Not sure. I thought it might be useful if you were setting up a connection to a foreign server and you knew that you only wanted read access to all the tables in it, this would avoid having to specify the option on every table. Regards, Dean
Dean Rasheed <dean.a.rasheed@gmail.com> writes: > [ pg_relation_is_updatable.patch ] I've committed this with some modifications as mentioned. There is still room to debate exactly what information_schema.columns.is_updatable means --- we can now change that without an initdb. regards, tom lane
On 12 June 2013 23:01, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Dean Rasheed <dean.a.rasheed@gmail.com> writes: >> [ pg_relation_is_updatable.patch ] > > I've committed this with some modifications as mentioned. There is > still room to debate exactly what > information_schema.columns.is_updatable means --- we can now change that > without an initdb. > Thanks. Those modifications all look pretty neat. I'm inclined to stick with the current definition of what updatable means in the information schema. I'm not entirely convinced that other possible interpretations of the spec are any more valid, and it certainly doesn't seem worth another initdb or a break with backwards compatibility by changing it. At least we now have functions that can give a more intuitive result for updatability. Regards, Dean
On 6/13/13 1:37 AM, Dean Rasheed wrote: > On 12 June 2013 23:01, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> > Dean Rasheed <dean.a.rasheed@gmail.com> writes: >>> >> [ pg_relation_is_updatable.patch ] >> > >> > I've committed this with some modifications as mentioned. There is >> > still room to debate exactly what >> > information_schema.columns.is_updatable means --- we can now change that >> > without an initdb. >> > > Thanks. Those modifications all look pretty neat. We still don't have any support for this in psql, do we?
On 19 June 2013 15:22, Peter Eisentraut <peter_e@gmx.net> wrote: > We still don't have any support for this in psql, do we? > No, but at least we now have an API that psql can use. There are still a number of questions about the best way to display it in psql. Should it be another column in \d+'s list of relations? Should it appear in \d+ for a single relation? Should it distinguish updatable from insertable and deletable? Should tab-completion also be modified? Currently I'm thinking yes, yes, no, yes. Regards, Dean
On 6/19/13 11:50 AM, Dean Rasheed wrote: > On 19 June 2013 15:22, Peter Eisentraut <peter_e@gmx.net> wrote: >> We still don't have any support for this in psql, do we? >> > > No, but at least we now have an API that psql can use. > > There are still a number of questions about the best way to display it in psql. > Should it be another column in \d+'s list of relations? > Should it appear in \d+ for a single relation? > Should it distinguish updatable from insertable and deletable? > Should tab-completion also be modified? > > Currently I'm thinking yes, yes, no, yes. I would be satisfied with no, yes, no, no. Although I don't know what tab completion changes you have in mind.
On 19 June 2013 18:12, Peter Eisentraut <peter_e@gmx.net> wrote: > On 6/19/13 11:50 AM, Dean Rasheed wrote: >> On 19 June 2013 15:22, Peter Eisentraut <peter_e@gmx.net> wrote: >>> We still don't have any support for this in psql, do we? >>> >> >> No, but at least we now have an API that psql can use. >> >> There are still a number of questions about the best way to display it in psql. >> Should it be another column in \d+'s list of relations? >> Should it appear in \d+ for a single relation? >> Should it distinguish updatable from insertable and deletable? >> Should tab-completion also be modified? >> >> Currently I'm thinking yes, yes, no, yes. > > I would be satisfied with no, yes, no, no. Although I don't know what > tab completion changes you have in mind. > Yes, on reflection having an extra column in the list of relations is probably not a good idea. In many cases that's just going to be a list of tables, all of which will be updatable. So it would only be for \d+ on a single view or foreign table - simply: Updatable: yes|no Tab-completion was discussed on the original thread, but then I forgot about it: http://www.postgresql.org/message-id/CAA-aLv4_atXiJ7pAQGvh73N5A0F-paTvH5eM-LMqu+oFuzE63w@mail.gmail.com Regards, Dean
On 19 June 2013 18:12, Peter Eisentraut <peter_e@gmx.net> wrote: > On 6/19/13 11:50 AM, Dean Rasheed wrote: >> On 19 June 2013 15:22, Peter Eisentraut <peter_e@gmx.net> wrote: >>> We still don't have any support for this in psql, do we? >>> >> >> No, but at least we now have an API that psql can use. >> >> There are still a number of questions about the best way to display it in psql. >> Should it be another column in \d+'s list of relations? >> Should it appear in \d+ for a single relation? >> Should it distinguish updatable from insertable and deletable? >> Should tab-completion also be modified? >> >> Currently I'm thinking yes, yes, no, yes. > > I would be satisfied with no, yes, no, no. Although I don't know what > tab completion changes you have in mind. > Here's a patch that does that for foreign tables and views. It regards "updatable" as support for *any* of the DML operations. Bernd Helmle has written a patch for tab completion. Regards, Dean