Re: Fix search_path for all maintenance commands

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>
Cc: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Gurjeet Singh <gurjeet(at)singh(dot)im>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: Fix search_path for all maintenance commands
Date: 2023-07-21 22:32:43
Message-ID: a5960b98b7d8b655f46b4b69387f1290f5848ad4.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 2023-07-17 at 12:16 -0700, Jeff Davis wrote:
> Based on feedback, I plan to commit soon.

Attached is a new version.

Changes:

* Also switch the search_path during CREATE MATERIALIZED VIEW, so that
it's consistent with REFRESH. As a part of this change, I slightly
reordered things in ExecCreateTableAs() so that the skipData path
returns early without entering the SECURITY_RESTRICTED_OPERATION. I
don't think that's a problem because (a) that is one place where
SECURITY_RESTRICTED_OPERATION is not used for security, but rather for
consistency; and (b) that path doesn't go through rewriter, planner, or
executor anyway so I don't see why it would matter.

* Use GUC_ACTION_SAVE rather than GUC_ACTION_SET. That was a problem
with the previous patch for index functions executed in parallel
workers, which can happen calling SQL functions from pg_amcheck.

* I used a wrapper function RestrictSearchPath() rather than calling
set_config_option() directly. That provides a nice place in case we
need to add a compatibility GUC to disable it.

Question:

Why do we switch to the table owner and use
SECURITY_RESTRICTED_OPERATION in DefineIndex(), when we will switch in
index_build (etc.) anyway? Similarly, why do we switch in vacuum_rel(),
when it doesn't matter for lazy vacuum and we will switch in
cluster_rel() and do_analyze_rel() anyway?

For now, I left the extra calls to RestrictSearchPath() in for
consistency with the switches to the table owner.

Regards,
Jeff Davis

Attachment Content-Type Size
v1-0001-Restrict-search_path-when-performing-maintenance.patch text/x-patch 31.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2023-07-21 23:14:12 Re: Row pattern recognition
Previous Message Melanie Plageman 2023-07-21 22:30:06 Re: Show WAL write and fsync stats in pg_stat_io