From: Michael Paquier <michael@paquier.xyz>
To: Christophe Pettus <xof@thebuild.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: vaibhave postgres <postgresvaibhave@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Noah Misch <noah@leadboat.com>
Cc: pgsql-bugs@lists.postgresql.org
Cc: vsekar@microsoft.com
Subject: Re: vacuumdb: permission denied for schema "pg_temp_7"
Date: Tue, 24 Sep 2024 10:08:43 +0900
Message-ID: <ZvIRG6282T33q3Ku@paquier.xyz> (raw)
In-Reply-To: <E9660D68-5B26-488E-9AB2-2CA1E8EDDDA1@thebuild.com>
References: <CAM_eQjwfAR=y3G1fGyS1U9FTmc+FyJm9amNfY2QCZBnDDbNPZg@mail.gmail.com>
<20240920190731.37.nmisch@google.com>
<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>
On Mon, Sep 23, 2024 at 11:48:21AM -0700, Christophe Pettus wrote:
> I'm happy to pick it up iff the current patch submitter doesn't want
> to continue with it.
Somewhat missed this thread, thanks for the latest activity.
If we apply a restriction on the temporary persistence, then we know
that vacuumdb will always have a WHERE clause so we can simplify the
code and remove the business with has_where like in the attached.
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.
The REINDEX one is really something that we need to care about? One
needs database ownership for a database-level REINDEX, or schema-level
ownership for the schema-level one. Stricter restrictions apply
compared to vacuum_rel().
Side note: we could use more CppAsString2() for relpersistence in
src/bin/, like in pg_amcheck, if we go this way.
--
Michael
Attachments:v2-0001-vacuumdb-Skip-temporary-tables-in-filtering-query.patch (text/x-diff, 3.9K) download
From a234eb7a0ad24a9d5641168346ce608515f484c2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 24 Sep 2024 10:00:29 +0900
Subject: [PATCH v2] vacuumdb: Skip temporary tables in filtering query
Running vacuumdb with a non-superuser when another user has created a
temporary table would lead to a mid-flight failure.
There are other issues related to schema-level USAGE, which will be
tackled later.
Author: VaibhaveS
Discussion: https://postgr.es/m/CAM_eQjwfAR=y3G1fGyS1U9FTmc+FyJm9amNfY2QCZBnDDbNPZg@mail.gmail.com
Backpatch-through: 12
---
src/bin/scripts/vacuumdb.c | 40 ++++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.cindex 7c33e13e1a..249622068a 100644--- a/src/bin/scripts/vacuumdb.c+++ b/src/bin/scripts/vacuumdb.c@@ -491,7 +491,6 @@ vacuum_one_database(ConnParams *cparams,
int ntups;
bool failed = false;
bool objects_listed = false;
- bool has_where = false;
const char *initcmd;
const char *stage_commands[] = {
"SET default_statistics_target=1; SET vacuum_cost_delay=0;",
@@ -665,7 +664,10 @@ vacuum_one_database(ConnParams *cparams,
" LEFT JOIN pg_catalog.pg_class t"
" ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n");
- /* Used to match the tables or schemas listed by the user */+ /*+ * Used to match the tables or schemas listed by the user, completing the+ * JOIN clause.+ */
if (objects_listed)
{
appendPQExpBufferStr(&catalog_query, " LEFT JOIN listed_objects"
@@ -676,14 +678,26 @@ vacuum_one_database(ConnParams *cparams,
appendPQExpBufferStr(&catalog_query, "c.oid\n");
else
appendPQExpBufferStr(&catalog_query, "ns.oid\n");
+ }+ /*+ * Exclude temporary tables, beginning the WHERE clause.+ */+ appendPQExpBufferStr(&catalog_query,+ " WHERE c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP));++ /*+ * Used to match the tables or schemas listed by the user, for the WHERE+ * clause.+ */+ if (objects_listed)+ {
if (objfilter & OBJFILTER_SCHEMA_EXCLUDE)
appendPQExpBuffer(&catalog_query,
- " WHERE listed_objects.object_oid IS NULL\n");+ " AND listed_objects.object_oid IS NULL\n");
else
appendPQExpBuffer(&catalog_query,
- " WHERE listed_objects.object_oid IS NOT NULL\n");- has_where = true;+ " AND listed_objects.object_oid IS NOT NULL\n");
}
/*
@@ -695,11 +709,9 @@ vacuum_one_database(ConnParams *cparams,
if ((objfilter & OBJFILTER_TABLE) == 0)
{
appendPQExpBuffer(&catalog_query,
- " %s c.relkind OPERATOR(pg_catalog.=) ANY (array["+ " AND c.relkind OPERATOR(pg_catalog.=) ANY (array["
CppAsString2(RELKIND_RELATION) ", "
- CppAsString2(RELKIND_MATVIEW) "])\n",- has_where ? "AND" : "WHERE");- has_where = true;+ CppAsString2(RELKIND_MATVIEW) "])\n");
}
/*
@@ -712,25 +724,23 @@ vacuum_one_database(ConnParams *cparams,
if (vacopts->min_xid_age != 0)
{
appendPQExpBuffer(&catalog_query,
- " %s GREATEST(pg_catalog.age(c.relfrozenxid),"+ " AND GREATEST(pg_catalog.age(c.relfrozenxid),"
" pg_catalog.age(t.relfrozenxid)) "
" OPERATOR(pg_catalog.>=) '%d'::pg_catalog.int4\n"
" AND c.relfrozenxid OPERATOR(pg_catalog.!=)"
" '0'::pg_catalog.xid\n",
- has_where ? "AND" : "WHERE", vacopts->min_xid_age);- has_where = true;+ vacopts->min_xid_age);
}
if (vacopts->min_mxid_age != 0)
{
appendPQExpBuffer(&catalog_query,
- " %s GREATEST(pg_catalog.mxid_age(c.relminmxid),"+ " AND GREATEST(pg_catalog.mxid_age(c.relminmxid),"
" pg_catalog.mxid_age(t.relminmxid)) OPERATOR(pg_catalog.>=)"
" '%d'::pg_catalog.int4\n"
" AND c.relminmxid OPERATOR(pg_catalog.!=)"
" '0'::pg_catalog.xid\n",
- has_where ? "AND" : "WHERE", vacopts->min_mxid_age);- has_where = true;+ vacopts->min_mxid_age);
}
/*
--
2.45.2
signature.asc (application/pgp-signature, 832B) download
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-bugs@postgresql.org
Cc: michael@paquier.xyz, xof@thebuild.com, nathandbossart@gmail.com, postgresvaibhave@gmail.com, tgl@sss.pgh.pa.us, masao.fujii@oss.nttdata.com, noah@leadboat.com, pgsql-bugs@lists.postgresql.org, vsekar@microsoft.com
Subject: Re: vacuumdb: permission denied for schema "pg_temp_7"
In-Reply-To: <ZvIRG6282T33q3Ku@paquier.xyz>
* 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