Re: Why not install pgstattuple by default? - Mailing list pgsql-hackers

On Sun, May 8, 2011 at 12:02 AM, Greg Smith <greg@2ndquadrant.com> wrote:
> Attached patch is a first cut at what moving one contrib module (in this
> case pg_buffercache) to a new directory structure might look like.  The idea
> is that src/extension could become a place for "first-class" extensions to
> live.  Those are ones community is committed to providing in core, but are
> just better implemented as extensions than in-database functions, for
> reasons that include security.  This idea has been shared by a lot of people
> for a while, only problem is that it wasn't really practical to implement
> cleanly until the extensions code hit.  I think it is now, this attempts to
> prove it.
>
> Since patches involving file renaming are clunky, the changes might be
> easier to see at
> https://github.com/greg2ndQuadrant/postgres/commit/507923e21e963c873a84f1b850d64e895776574f
> where I just pushed this change too.  The install step for the modules looks
> like this now:
>
> gsmith@grace:~/pgwork/src/move-contrib/src/extension/pg_buffercache$ make
> install
> /bin/mkdir -p '/home/gsmith/pgwork/inst/move-contrib/lib/postgresql'
> /bin/mkdir -p
> '/home/gsmith/pgwork/inst/move-contrib/share/postgresql/extension'
> /bin/sh ../../../config/install-sh -c -m 755  pg_buffercache.so
> '/home/gsmith/pgwork/inst/move-contrib/lib/postgresql/pg_buffercache.so'
> /bin/sh ../../../config/install-sh -c -m 644 ./pg_buffercache.control
> '/home/gsmith/pgwork/inst/move-contrib/share/postgresql/extension/'
> /bin/sh ../../../config/install-sh -c -m 644 ./pg_buffercache--1.0.sql
> ./pg_buffercache--unpackaged--1.0.sql
>  '/home/gsmith/pgwork/inst/move-contrib/share/postgresql/extension/'
> $ psql -c "create extension pg_buffercache"
> CREATE EXTENSION
>
> The only clunky bit I wasn't really happy with is the amount of code
> duplication that comes from having a src/extension/Makefile that looks
> almost, but not quite, identical to contrib/Makefile.  The rest of the
> changes don't seem too bad to me, and even that's really only 36 lines that
> aren't touched often.  Yes, the paths are different, so backports won't
> happen without an extra step.  But the code changes required were easier
> than I was expecting, due to the general good modularity of the extensions
> infrastructure.  So long as the result ends up in
> share/postgresql/extension/ , whether they started in contrib/<module> or
> src/extension/<module> doesn't really matter to CREATE EXTENSION.  But
> having them broke out this way makes it easy for the default Makefile to
> build and install them all.  (I recognize I didn't do that last step yet
> though)
>
> I'll happily go covert pgstattuple and the rest of the internal diagnostics
> modules to this scheme, and do the doc cleanups, this upcoming week if it
> means I'll be able to use those things without installing all of contrib one
> day.  Ditto for proposing RPM and Debian packaging changes that match them.
>  All that work will get paid back the first time I don't have to fill out a
> bunch of paperwork (again) at a customer site justifying why they need to
> install the contrib [RPM|deb] package (which has some scary stuff in it) on
> all their servers, just so I can get some bloat or buffer inspection module.

I would really like to see us try to group things by topic, and not
just by whether or not we can all agree that the extension is
important enough to be first-class (which is bound to be a bit
tendentious).  We probably can't completely avoid some bikeshedding on
that topic, but even there it strikes me that sorting by topic might
make things a bit more clear.  For example, if we could somehow group
together all the diagnostic tools, maybe something like the list
below, I think that would be a start.  Now then we might go on to
argue about which are the more useful diagnostic tools, but I think
it's easier to argue about that category than it is to argue in the
abstract about whether you'd rather have hstore or pgstattuple, to
which the answer can only be "that depends".

auto_explain
oid2name
pageinspect
pg_buffercache
pg_freespacemap
pg_stat_statements
pg_test_fsync (perhaps)
pgrowlocks
pgstattuple

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