Horton

Re: vacuumdb: permission denied for schema "pg_temp_7"

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Christophe Pettus <xof@thebuild.com>, Nathan Bossart <nathandbossart@gmail.com>, vaibhave postgres <postgresvaibhave@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>, pgsql-bugs@lists.postgresql.org, vsekar@microsoft.com
Date: 2024-09-24T23:20:22Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. vacuumdb: Schema-qualify operator in catalog query's WHERE clause.

  2. reindexdb: Skip reindexing temporary tables and indexes.

  3. vacuumdb: Skip temporary tables in query to build list of relations

  4. Use catalog query to discover tables to process in vacuumdb

On Tue, Sep 24, 2024 at 11:20:43PM +0900, Fujii Masao wrote:
> On 2024/09/24 10:08, Michael Paquier wrote:
>> About the permission restrictions depending on the objects listed, the
>> filtering query uses currently a list of VALUES in a CTE.  Perhaps it
>> would be more elegant to switch that to a SELECT with some
>> has_schema_privilege() for the cases where OBJFILTER_SCHEMA is
>> used?
>> 
>> There permission checks with USAGE and MAINTAIN are broader, so I'd
>> choose to add a skip on the temp persistence first and backpatch it
>> down to 12 as there is also a performance argument.  Then tackle the
>> rest by reworking the VALUES part in the CTE.
> 
> Are you suggesting that any objects a user lacks sufficient privileges for
> should be silently excluded from vacuuming? This could make vacuumdb appear
> successful because no errors occur, but some tables the user intended to
> vacuum might be skipped without notice. That seems more problematic to me.
> 
> Sorry if I misunderstood your point.

I don't really have a strong opinion in favor of skipping the objects
or fail hard, because both have arguments.  My point was just that, if
we were to skip the objects, then the query needs a bit of rework and
a SELECT with some extra filters in the CTE felt kind of right.  If
the consensus is that errors are better, my point about rewriting the
query is most probably moot.

> Is ownership really necessary in these cases? A similar issue can easily
> happen with reindexdb as follows, so I think that should be fixed as well.
> 
> =# CREATE ROLE admin WITH LOGIN;
> =# GRANT pg_maintain TO admin;
> =# CREATE TEMP TABLE tt (i int primary key);
> =# \! bin/reindexdb -U admin -j 2 postgres
> reindexdb: error: query failed: ERROR:  permission denied for schema pg_temp_0
> LINE 4:   AND c.oid OPERATOR(pg_catalog.=) 'pg_temp_0.tt'::pg_catalo...
>                                            ^
> reindexdb: detail: Query was: SELECT c.relname, ns.nspname
>  FROM pg_catalog.pg_class c, pg_catalog.pg_namespace ns
>  WHERE c.relnamespace OPERATOR(pg_catalog.=) ns.oid
>   AND c.oid OPERATOR(pg_catalog.=) 'pg_temp_0.tt'::pg_catalog.regclass;
> 
> We should probably add a condition like "relpersistence != CppAsString2(RELPERSISTENCE_TEMP)"
> to the queries in get_parallel_object_list().

Missed your point.  An extra filter based on relpersistence can indeed
make sense for this path.
--
Michael