From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tomas Vondra <tomas.vondra@postgresql.org>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Rework the pg_statistic_ext catalog
Date: Sun, 16 Jun 2019 12:03:28 +0200
Message-ID: <20190616100328.shl2hhuy2pup6ges@development> (raw)
In-Reply-To: <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>
<20190616085651.d2hxuiqlojewed5j@development>
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, <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).
>
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.
But we're not out of the woods yet, it seems. check-world still fails
for me with an error like this:
t/011_crash_recovery.pl .............. 1/3
# Failed test 'new xid after restart is greater'
# at t/011_crash_recovery.pl line 61.
# '486'
# >
# '486'
# Failed test 'xid is aborted after crash'
# at t/011_crash_recovery.pl line 65.
# got: 'committed'
# expected: 'aborted'
# Looks like you failed 2 tests of 3.
t/011_crash_recovery.pl .............. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/3 subtests
which I find rather strange. I have no idea why splitting a catalog and
adding a system view should affect recovery like this.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, dean.a.rasheed@gmail.com, tgl@sss.pgh.pa.us, tomas.vondra@postgresql.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Rework the pg_statistic_ext catalog
In-Reply-To: <20190616100328.shl2hhuy2pup6ges@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox