On Sun, Jun 16, 2019 at 12:03:28PM +0200, Tomas Vondra wrote:
On Sun, Jun 16, 2019 at 10:56:51AM +0200, Tomas Vondra wrote:
On Sun, Jun 16, 2019 at 07:18:58AM +0100, Dean Rasheed wrote:
On Sun, 16 Jun 2019, 03:05 Tom Lane, <[email protected]> wrote:
I wrote:
Tomas Vondra <[email protected]> writes:
Rework the pg_statistic_ext catalog
So ... not one of the buildfarm members that are running TAP tests
likes this. ...
I think probably what's happening is that pg_dump is still trying to dump
directly from the catalog, when what it needs to do now is dump from the
view, in case it's not running as superuser.
I experimented with extracting the required data from the view, and
there are at least two show-stopper problems:
* The view doesn't expose pg_statistic_ext.oid, which pg_dump has to have
for dependency tracking purposes. I think we could just add it though.
Now that OIDs are ordinary columns it won't even look very odd.
* Rather than just not exposing the critical data for stats you don't
have privilege to read, the view doesn't expose anything at all.
I do not think that's acceptable; it creates a significant hazard of
data loss during pg_dump, for no very good reason. What we should
be doing, IMO, is still showing all the rows but filling the data-value
columns with nulls in rows where the caller can't access the underlying
data.
Hang on. Isn't the real problem that we should be revoking public access
from pg_statistic_ext_data, not pg_statistic_ext? Normal users should still
be able to see the stats definitions in the catalog table, but not the
stats data, so I think pg_dump wouldn't need changing.
Yeah, that's pretty much why we split the catalog - to allow pg_dump to
read the definitions, without exposing the stats too. I'll look into
this and push a fix to unbreak the buildfarm (sorry about that).
OK, the bug here seems to be pretty silly - the GRANT in system_views
does this:
GRANT SELECT (oid, stxrelid, stxname, stxnamespace, stxowner, stxkeys, stxkind)
ON pg_statistic_ext TO public;
but pg_dump also uses tableoid column. After adding it to the GRANT I
can run pg_dump using regular user just fine. I'll push this fix
shortly.
OK, it seems the buildfarm is getting green again. I'm not sure why I'm
seeing the failure in 011_crash_recovery.pl, while the buildfarm does
not. Perhaps my environment is borked in some way? Not sure.
Anyway, I'm wondering if we need the REVOKE/GRANT on pg_statistic_ext at
all, and if we do if we should just remove the list of columns, and
grant SELECT on all. All the sensitive columns were moved to another
catalog, after all.
That also reminds me the patch should have revoked access to the new
_data catalog.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
