Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 25 Sep 2024 08:20:22 +0900
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
Subject: Re: vacuumdb: permission denied for schema "pg_temp_7"
Message-ID: <ZvNJNoumGREYshTF@paquier.xyz>
References: <Zu3iMzfiGBTbg3iy@nathan>
 <20240920215030.8d.nmisch@google.com>
 <2603142.1726873660@sss.pgh.pa.us>
 <5f37ee56-14fb-44fe-9150-9eb97e10538b@oss.nttdata.com>
 <2649165.1726898595@sss.pgh.pa.us>
 <CAM_eQjyWnu_LUt6JWjh3boNFHAT9DBrcbamhG3Xy39Q4Ph=2rg@mail.gmail.com>
 <ZvGye19tHrhLCpFl@nathan>
 <E9660D68-5B26-488E-9AB2-2CA1E8EDDDA1@thebuild.com>
 <ZvIRG6282T33q3Ku@paquier.xyz>
 <00d98a9f-bec1-4cd0-b59e-1043ae04c8bc@oss.nttdata.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="xmt1f+3PxrVpsfSV"
Content-Disposition: inline
In-Reply-To: <00d98a9f-bec1-4cd0-b59e-1043ae04c8bc@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZvNJNoumGREYshTF%40paquier.xyz>
Precedence: bulk


--xmt1f+3PxrVpsfSV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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?
>>=20
>> 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.
>=20
> Are you suggesting that any objects a user lacks sufficient privileges for
> should be silently excluded from vacuuming? This could make vacuumdb appe=
ar
> successful because no errors occur, but some tables the user intended to
> vacuum might be skipped without notice. That seems more problematic to me.
>=20
> 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.
>=20
> =3D# CREATE ROLE admin WITH LOGIN;
> =3D# GRANT pg_maintain TO admin;
> =3D# CREATE TEMP TABLE tt (i int primary key);
> =3D# \! bin/reindexdb -U admin -j 2 postgres
> reindexdb: error: query failed: ERROR:  permission denied for schema pg_t=
emp_0
> LINE 4:   AND c.oid OPERATOR(pg_catalog.=3D) '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.=3D) ns.oid
>   AND c.oid OPERATOR(pg_catalog.=3D) 'pg_temp_0.tt'::pg_catalog.regclass;
>=20
> We should probably add a condition like "relpersistence !=3D 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

--xmt1f+3PxrVpsfSV
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmbzSTYACgkQnvQgOdby
QH2O8Q//dfyoaYHrIr0x4meS7jD/ipiYpCKtkiBV/eKJESF3+ossOZZJOMSwHmCK
SJHAw0nv175Z6iZmm7cBDQGdPObvMuHLQRl7rfegZEBhNOD9eFtGktmlJbbAG/Co
gdnoCkdlbRu2GVhZyMbYyFZ8+3LuMQNxZrzObFkdB2XSs9SnZcA0q0dQoc5S2VmM
xhLDoo4pP0GWhOWhuyshHkxQ8cWqpuakZYzyAXS9oQjSJs+C3vhYODKFw0GhAqru
+JKyQhgNgTySnlxKXvVISPOmsAaaCIZV5OEX4PpqXFUzRBAQvRS++AiWmKH43kx8
UV7VaMtCkCKa0rhdWjGH4VFyMARSo3+31vVJ1qx0D+M1ADD8SroWW6ume7PbLAbc
TuDpptYja6VM4Xnbn62Kho3iIuvvis0kgg6ClCb6CVDH1RLCFsZLkrGi/BiEsx1m
ieRh01RXqAnTPsN38FU9Ekmgg295cqJBSmp6k+utgiPTJCFikGrrdoP7hZHQecEK
vmta18QvzFTZuyP0TGWc6AcCKuc7aTGWITwQPUyUygORKZw6Yspkf5q9l+XGmY9O
VenRXzF5m9GeuMmPgtBWSuGT0gu14B/Kj5O9mIgyuFa/eESLcCdREwEfBcc/rLdU
7RrM9ErylPDm5jYUE3mqKjATxQQuooJUcz07RxFeMfDw3JjLn44=
=6230
-----END PGP SIGNATURE-----

--xmt1f+3PxrVpsfSV--