Date: Sun, 16 Jun 2019 10:56:51 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	Tomas Vondra <tomas.vondra@postgresql.org>,
	pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Rework the pg_statistic_ext catalog
Message-ID: <20190616085651.d2hxuiqlojewed5j@development>
References: <E1hcHz2-0005mY-5A@gemulon.postgresql.org>
 <8833.1560647898@sss.pgh.pa.us>
 <20156.1560650744@sss.pgh.pa.us>
 <CAEZATCWWX5O57d=MHY=MqyJe-xHG2pbi75LKLjo6aZLP3zsaow@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: 
 <CAEZATCWWX5O57d=MHY=MqyJe-xHG2pbi75LKLjo6aZLP3zsaow@mail.gmail.com>
User-Agent: NeoMutt/20180716-1444-295967
Precedence: bulk

On Sun, Jun 16, 2019 at 07:18:58AM +0100, Dean Rasheed wrote:
>On Sun, 16 Jun 2019, 03:05 Tom Lane, <tgl@sss.pgh.pa.us> wrote:
>
>> I wrote:
>> > Tomas Vondra <tomas.vondra@postgresql.org> 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).

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services