On Tue, 2024-07-09 at 15:20 +0900, Michael Paquier wrote:
> On Sun, Jun 30, 2024 at 03:23:44PM -0700, Noah Misch wrote:
> > I've audited NewGUCNestLevel() calls that didn't get this
> > addition. Among
> > those, these need the addition:
> >
> > - Each in ComputeIndexAttrs() -- they arise when the caller is
> > DefineIndex()
> > - In DefineIndex(), after comment "changed a behavior-affecting
> > GUC"
Thank you for the report. Patch attached to address these missing call
sites.
> Hmm. Is RestrictSearchPath() something that we should advertise more
> strongly, thinking here about extensions that call NewGUCNestLevel()?
> That would be really easy to miss, and it could have bad
> consequences.
> I know that this is not something that's published in the release
> notes, but it looks like something sensible to have, though.
The pattern also involves SetUserIdAndSecContext(). Perhaps we could
come up with a wrapper function to better encapsulate the general
pattern?
> > While "not necessary for security", ExecCreateTableAs() should do
> > it for the
> > same reason it calls NewGUCNestLevel().
>
> +1.
Do you have a suggestion about how that should be done?
It's not trivial, because the both creates the table and populates it
in ExecutorRun. For table creation, we need to use the original
search_path, but we need to use the restricted search_path when
populating it.
I could try to refactor it into two statements and execute them
separately, or I could try to rewrite the statement to use a fully-
qualified destination table before execution. Thoughts?
Regards,
Jeff Davis
