Обсуждение: adminpack and pg_catalog
Why does adminpack install functions into pg_catalog? This is inconsistent with the rest of the contrib/ packages, not to mention the definition of pg_catalog itself (which ought to hold builtin object definitions). And as AndrewSN pointed out on IRC, it also breaks pg_dump. -Neil
On Thu, Oct 19, 2006 at 02:37:34PM -0400, Neil Conway wrote: > Why does adminpack install functions into pg_catalog? This is > inconsistent with the rest of the contrib/ packages, not to mention the > definition of pg_catalog itself (which ought to hold builtin object > definitions). And as AndrewSN pointed out on IRC, it also breaks > pg_dump. When we wrote newsysviews we faced the same dilema of where to put things. We ultimately decided on a pg_ schema in the hope that no one else would use that name (we also prefaced everything in the schema with pg_, but a big reason for that was potential inclusion into core). -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
On 19/10/06 19:37, "Neil Conway" <neilc@samurai.com> wrote: > Why does adminpack install functions into pg_catalog? This is > inconsistent with the rest of the contrib/ packages, not to mention the > definition of pg_catalog itself (which ought to hold builtin object > definitions). The adminpack was originally written and intended to become builtin functions, and in fact most of it has. pg_catalog was used to ensure compatibility in the future, and because it's the only schema we can be sure we'll find in any given database. > And as AndrewSN pointed out on IRC, it also breaks > pg_dump. It does? In what way? I don't recall ever having any trouble restoring dumps in the past. Regards, Dave.
Neil Conway wrote: > Why does adminpack install functions into pg_catalog? This is > inconsistent with the rest of the contrib/ packages, not to mention the > definition of pg_catalog itself (which ought to hold builtin object > definitions). And as AndrewSN pointed out on IRC, it also breaks > pg_dump. > Having pg_dump not saving the function definitions is an intended behaviour. Actually, this was different with admin80, and restoring a 8.0 backup to a 8.1 server will throw several errors now. I'd consider installing contrib modules as an act of installation, not something that backup/restore should perform (finally, pg_restore isn't able to do so, since it can't provide the dll/lib module). Regards, Andreas
On Fri, 2006-10-20 at 11:50 +0200, Andreas Pflug wrote: > Having pg_dump not saving the function definitions is an intended > behaviour. The manual defines the pg_catalog schema as containing "the system tables and all the built-in data types, functions, and operators" (section 5.7.5). adminpack is none of the above, so I don't think it should be located in pg_catalog. > I'd consider installing contrib modules as an act of installation, not > something that backup/restore should perform AFAICS this is inconsistent with how every other contrib module behaves: installing the contrib module into a database results in DDL for that contrib module being included in pg_dump's output. > (finally, pg_restore isn't able to do so, since it can't provide > the dll/lib module). This is not related to adminpack per se: pg_dump is never be able to provide the shared object for any C language UDF. By your logic, pg_dump shouldn't emit DDL for any such function. -Neil
On Fri, 2006-10-20 at 05:52 +0100, Dave Page wrote: > The adminpack was originally written and intended to become builtin > functions This is not unique to adminpack: several contrib modules might eventually become (or have already become) builtins, but adminpack is the only module that defines objects in the pg_catalog schema. > pg_catalog was used to ensure compatibility in the future This is again not unique to adminpack. If users install a contrib module into a schema that is in their search path, then if the module is subsequently moved to pg_catalog, no queries will need to be changed. If users install a module into some schema that isn't in their search path and use explicit schema references, they are essentially asking for their application to break if the object moves to a different schema. > > And as AndrewSN pointed out on IRC, it also breaks pg_dump. > > It does? In what way? It breaks in the sense of "completely not working" :) % pg_dump | grep file_write % cd contrib/adminpack % grep -A1 file_write adminpack.sql CREATE FUNCTION pg_catalog.pg_file_write(text, text, bool) RETURNS bigint AS '$libdir/adminpack', 'pg_file_write'LANGUAGE C VOLATILE STRICT; % psql -f adminpack.sql CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION CREATE FUNCTION % pg_dump | grep file_write % -Neil
Neil Conway wrote: > Why does adminpack install functions into pg_catalog? This is > inconsistent with the rest of the contrib/ packages, not to mention > the definition of pg_catalog itself (which ought to hold builtin > object definitions). Nothing except initdb should add objects in pg_catalog. AFAICS, adminpack doesn't have any special requirements, so it should behave like all other contrib modules. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Neil Conway wrote: > On Fri, 2006-10-20 at 05:52 +0100, Dave Page wrote: > >> The adminpack was originally written and intended to become builtin >> functions >> > > This is not unique to adminpack: several contrib modules might > eventually become (or have already become) builtins, but adminpack is > the only module that defines objects in the pg_catalog schema. > .. which appears simply pragmatic, taken that it features server maintenance functions, not functions usually called from user applications. > >> pg_catalog was used to ensure compatibility in the future >> > > This is again not unique to adminpack. If users install a contrib module > into a schema that is in their search path, then if the module is > subsequently moved to pg_catalog, no queries will need to be changed. If > users install a module into some schema that isn't in their search path > and use explicit schema references, they are essentially asking for > their application to break if the object moves to a different schema. > Please note that adminpack is intended for administrator's use, and should be robust to (i.e. not dependent on) search path. We previously had this dependency in pgadmin, and found it sucks. Putting the stuff in pg_catalog works as desired and has no negative effects (apart from the contrib not working after pg_dump/pg_restore if not installed, which is expected behaviour anyway). However, adminpack was crippled to the edge of usability for me already, I'm prepared to see it fade away further (Since there's still no pg_terminate_backend available which is definitely needed, I regularly need to install my personal adminpack). Regards, Andreas
On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote: > Nothing except initdb should add objects in pg_catalog. AFAICS, > adminpack doesn't have any special requirements, so it should behave > like all other contrib modules. Okay. Are there any opinions on whether we should make this change to contrib/adminpack now (i.e. during the 8.2 beta), later (for 8.3), or not all at? -Neil
Neil Conway <neilc@samurai.com> writes:
> On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote:
>> Nothing except initdb should add objects in pg_catalog. AFAICS,
>> adminpack doesn't have any special requirements, so it should behave
>> like all other contrib modules.
> Okay. Are there any opinions on whether we should make this change to
> contrib/adminpack now (i.e. during the 8.2 beta), later (for 8.3), or
> not all at?
AFAIR the point of adminpack was to support pgAdmin, which expects those
functions to be in pg_catalog. At some point we might as well just take
it out instead of whack it until it meets some arbitrary restrictions
and isn't at all backwards-compatible anymore.
(No, I don't find these arguments that it mustn't put anything in
pg_catalog to be very compelling ... if we seriously believed that,
we'd have arranged for the system to enforce it.)
regards, tom lane
-----Original Message----- From: "Neil Conway" <neilc@samurai.com> To: "Peter Eisentraut" <peter_e@gmx.net> Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>; "Dave Page" <dpage@vale-housing.co.uk> Sent: 21/10/06 02:03 Subject: Re: [HACKERS] adminpack and pg_catalog On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote: > Nothing except initdb should add objects in pg_catalog. AFAICS, > adminpack doesn't have any special requirements, so it should behave > like all other contrib modules. > Okay. Are there any opinions on whether we should make this change to > contrib/adminpack now (i.e. during the 8.2 beta), later (for 8.3), or not all at? If you change it you will make it useless as pgAdmin won't necessarily find the functions it expects. You might as well justremove it (which will almost certainly cause delays to pgAdmin - and pgInstallers - release as I'll need to find timeto put it all back how it was). Regards, Dave
-----Original Message----- From: "Neil Conway" <neilc@samurai.com> To: "Dave Page" <dpage@vale-housing.co.uk> Cc: "PostgreSQL-development" <pgsql-hackers@postgresql.org> Sent: 20/10/06 21:19 Subject: Re: adminpack and pg_catalog > It breaks in the sense of "completely not working" :) No, it does not 'break pg_dump'. What you have shown is that pg_dump doesn't dump adminpack which is exactly what we want,and perfectly correct behaviour from pg_dump. Regards, Dave
Dave Page wrote: > If you change it you will make it useless as pgAdmin won't > necessarily find the functions it expects. You might as well just > remove it (which will almost certainly cause delays to pgAdmin - and > pgInstallers - release as I'll need to find time to put it all back > how it was). If pgAdmin is going to dictate what we do with adminpack, adminpack should rather be shipped with pgAdmin. -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Friday 20 October 2006 21:03, Neil Conway wrote:
> On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote:
> > Nothing except initdb should add objects in pg_catalog. AFAICS,
> > adminpack doesn't have any special requirements, so it should behave
> > like all other contrib modules.
>
> Okay. Are there any opinions on whether we should make this change to
> contrib/adminpack now (i.e. during the 8.2 beta), later (for 8.3), or
> not all at?
>
Not saying we should or shouldn't, but if we are going to do it, istm we ought
to do it now while we are still in beta. Is there a backwards compatability
issue for older systems? (Not that this doesn't get worse if we release yet
another release with it)
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
On Sat, 2006-10-21 at 12:37 +0200, Peter Eisentraut wrote: > Dave Page wrote: > > If you change it you will make it useless as pgAdmin won't > > necessarily find the functions it expects. You might as well just > > remove it (which will almost certainly cause delays to pgAdmin - and > > pgInstallers - release as I'll need to find time to put it all back > > how it was). > > If pgAdmin is going to dictate what we do with adminpack, adminpack > should rather be shipped with pgAdmin. Admin tools are critical. adminpack is being made available for any and all admin packages, so all tools, including for example psql *could* be enhanced to use those. Whether they do is up to them, but at least their life is potentially easier. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote: > Nothing except initdb should add objects in pg_catalog. AFAICS, > adminpack doesn't have any special requirements, so it should behave > like all other contrib modules. Where are we on this? When this topic was last discussed, the three alternatives were: (1) Modify contrib/adminpack to not use the pg_catalog schema, per the consensus that contrib/ packages installingobjects into that schema is broken behavior (2) Don't modify contrib/adminpack, for the sake of backward compatibility (3) Remove contrib/adminpack from the Postgres distribution I think the discussion was edging toward #3, but #2 is the only option that I'm not happy with. Any other opinions out there? Cheers, Neil
Neil Conway wrote: > On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote: >> Nothing except initdb should add objects in pg_catalog. AFAICS, >> adminpack doesn't have any special requirements, so it should behave >> like all other contrib modules. > > Where are we on this? When this topic was last discussed, the three > alternatives were:> > (1) Modify contrib/adminpack to not use the pg_catalog schema, > per the consensus that contrib/ packages installing objects > into that schema is broken behavior > > (2) Don't modify contrib/adminpack, for the sake of backward > compatibility > > (3) Remove contrib/adminpack from the Postgres distribution > > I think the discussion was edging toward #3, but #2 is the only option > that I'm not happy with. Any other opinions out there? Looking back over the thread, it appears that only you and Peter objected to it as it is now. Tom, Andreas and myself were of the opinion it was fine as it is, and whilst he didn't comment on how it should be implemented, Simon made the point that supporting admin tools from the core distribution was important which I take to mean he is against #3. Regards, Dave.
On Mon, 2006-11-06 at 09:02 +0000, Dave Page wrote: > Neil Conway wrote: > > On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote: > >> Nothing except initdb should add objects in pg_catalog. AFAICS, > >> adminpack doesn't have any special requirements, so it should behave > >> like all other contrib modules. > > > > Where are we on this? When this topic was last discussed, the three > > alternatives were: > > > > (1) Modify contrib/adminpack to not use the pg_catalog schema, > > per the consensus that contrib/ packages installing objects > > into that schema is broken behavior > > > > (2) Don't modify contrib/adminpack, for the sake of backward > > compatibility > > > > (3) Remove contrib/adminpack from the Postgres distribution > > > > I think the discussion was edging toward #3, but #2 is the only option > > that I'm not happy with. Any other opinions out there? > > Looking back over the thread, it appears that only you and Peter > objected to it as it is now. Tom, Andreas and myself were of the opinion > it was fine as it is, and whilst he didn't comment on how it should be > implemented, Simon made the point that supporting admin tools from the > core distribution was important which I take to mean he is against #3. Definitely against #3. [Argument: not just pgAdmin, essential feature] At the moment we only allow 2 types of table. Approved core catalog tables and user tables. ISTM we need 3 types of tables, with the additional type being add-on system functionality, such as adminpack, but also possibly including performance logging tables, extended trace tools, security packages (e.g. Veil) etc etc. We have made provision for user add-ons to allocate LWlocks and shared memory, but no provision for additional tables. Surely we must be able to accommodate things as simple as additional tables? So we would then have (change names as appropriate) - core catalog - catalog extensions - user tables. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> At the moment we only allow 2 types of table. Approved core catalog
> tables and user tables.
> ISTM we need 3 types of tables, with the additional type being add-on
> system functionality, such as adminpack,
What? The adminpack module only creates functions.
regards, tom lane
On Mon, 2006-11-06 at 13:37 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > > At the moment we only allow 2 types of table. Approved core catalog > > tables and user tables. > > > ISTM we need 3 types of tables, with the additional type being add-on > > system functionality, such as adminpack, > > What? The adminpack module only creates functions. AFAIK the namespace is the issue, not the type of object. As I mentioned, we can add LWlock extensions but we don't have an official home for other database objects. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On Monday 06 November 2006 13:12, Simon Riggs wrote:
> On Mon, 2006-11-06 at 09:02 +0000, Dave Page wrote:
> > Neil Conway wrote:
> > > On Fri, 2006-10-20 at 22:59 +0200, Peter Eisentraut wrote:
> > >> Nothing except initdb should add objects in pg_catalog. AFAICS,
> > >> adminpack doesn't have any special requirements, so it should behave
> > >> like all other contrib modules.
> > >
> > > Where are we on this? When this topic was last discussed, the three
> > > alternatives were:
> > >
> > > (1) Modify contrib/adminpack to not use the pg_catalog schema,
> > > per the consensus that contrib/ packages installing objects
> > > into that schema is broken behavior
> > >
> > > (2) Don't modify contrib/adminpack, for the sake of backward
> > > compatibility
> > >
> > > (3) Remove contrib/adminpack from the Postgres distribution
> > >
> > > I think the discussion was edging toward #3, but #2 is the only option
> > > that I'm not happy with. Any other opinions out there?
> >
> > Looking back over the thread, it appears that only you and Peter
> > objected to it as it is now. Tom, Andreas and myself were of the opinion
> > it was fine as it is, and whilst he didn't comment on how it should be
> > implemented, Simon made the point that supporting admin tools from the
> > core distribution was important which I take to mean he is against #3.
>
> Definitely against #3. [Argument: not just pgAdmin, essential feature]
>
While I don't disagree that this is an important feature, the fact that it is
being designed with pgadmin specific backwards compatability (for example the
functions that rename core functions) leaves me dubious as to it being a more
general solution. Because of that I would be comfortable with acting on #3.
Now, if I ignore the above, and focus on that I would like to see this
functionality because it helps me with phppgadmin, then I would lean toward
#1 (for a number of reasons really)
Personally I think I'd rather see the whole thing pulled, renamed to its own
schema, and toss in a version function and a kill backend function and let it
go on its merry way... in any case #2 just seems to be the worst of all
possibilities.
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
Robert Treat wrote: > While I don't disagree that this is an important feature, the fact that it is > being designed with pgadmin specific backwards compatability (for example the > functions that rename core functions) leaves me dubious as to it being a more > general solution. Because of that I would be comfortable with acting on #3. The rename happened because when one of the functions was moved into core, someone decided to rename it along the way which broke compatibility with previous versions of pgAdmin. Whilst I won't in any way deny that the adminpack was specifically written for pgAdmin though, I have no problem with it being enhanced to provide non-core functionality for other admin tools in the future... in fact I would encourage it. I think it's better to support as many admin tools as possible from one contrib module, rather than for each tool to have it's own. Regards Dave.