Re: Add Postgres module info

I spent awhile reviewing the v5 patch, and here's a proposed v6.
Some notes:

* I didn't like depending on offsetof(Pg_magic_struct, module_extra)
to determine which parts of the struct are checked for compatibility.
It just seems way too easy to break that with careless insertion
of new fields, and such breakage might not cause obvious failures.
I think the right thing is to break out the ABI-checking fields as
their own sub-struct, rather than breaking out the new fields as a
sub-struct.

* I renamed the inquiry function to pg_get_loaded_modules, since
it only works on loaded modules but that's hardly clear from the
previous name.

* It is not clear to me what permission restrictions we should put
on pg_get_loaded_modules, but it is clear that "none" is the wrong
answer.  In particular, exposing the full file path of loaded modules
is against our rules: unprivileged users are not supposed to be able
to learn anything about the filesystem underneath the server.  (This
is why for instance an unprivileged user can't read the data_directory
GUC.)  In the attached I made the library path read as NULL unless the
user has pg_read_server_files, but I'm not attached to that specific
solution.  One thing not to like is that it's very likely that you'd
just get a row of NULLs and no useful info about a module at all.
Another idea perhaps could be to strip off the directory path and
maybe the filename extension if the user doesn't have privilege.
Or we could remove the internal permission check and instead gate
access to the function altogether with grantable EXECUTE privilege.
(This might be the right answer, since it's not clear that Joe
Unprivileged User should be able to know what modules are loaded; some
of them might have security implications.)  In any case, requiring
pg_read_server_files feels a little too strong, but I don't see an
alternative role I like better.  The EXECUTE-privilege answer would at
least let installations adjust the function's availability to their
liking.

* I didn't like anything about the test setup.  Making test_misc
dependent on other modules is a recipe for confusion, and perhaps for
failures in parallel builds.  (Yes, I see somebody already made it
depend on injection_points.  But doubling down on a bad idea doesn't
make it less bad.)  Also, the test would fail completely in an
installation that came with any preloaded modules, which hardly seems
like an improbable future situation.  I think we need to restrict what
modules we're looking at with a WHERE clause to prevent that.  After
some thought I went back to the upthread idea of just having
auto_explain as a test case.

Still TBD:

* I'm not happy with putting pg_get_loaded_modules into dfmgr.c.
It feels like the wrong layer to have a SQL-callable function,
and the large expansion in its #include list is evidence that we're
adding functionality that doesn't belong there.  But I'm not quite
sure where to put it instead.  Also, the naive way to do that would
require exporting DynamicFileList which doesn't feel nice either.
Maybe we could make dfmgr.c export some sort of iterator function?

* Should we convert our existing modules to use PG_MODULE_MAGIC_EXT?
I'm mildly in favor of that, but I think we'd need some automated way
to manage their version strings, and I don't know what that ought to
look like.  Maybe it'd be enough to make all the in-core modules use
PG_VERSION as their version string, but I think that might put a dent
in the idea of the version strings following semantic versioning
rules.

			regards, tom lane