Hi Robert,
On Fri, Jul 12, 2024 at 9:02 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Mon, Jun 24, 2024 at 3:10 PM Jeff Davis <pg...@j-davis.com> wrote:
> > A special search_path variable "$extension_schema" would be a better
> > solution to this problem. We need something like that anyway, in case
> > the extension is relocated, so that we don't have to dig through the
> > catalog and update all the settings.
>
> +1. I think we need that in order for this proposal to make any progress.
>
> And it seems like the patch has something like that, but I don't
> really understand exactly what's going on here, because it's also got
> hunks like this in a bunch of places:
>
> + if (strcmp($2, "$extension_schema") == 0)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("search_path cannot be set to $extension_schema"),
> + parser_errposition(@2)));
>
> So it seems like the patch is trying to support $extension_schema in
> some cases but not others, which seems like an odd choice that, as far
> as I can see, is not explained anywhere: not in the commit message,
> not in the comments (which are pretty minimally updated by the patch),
> and not in the documentation (which the patch doesn't update at all).
>
> Ashutosh, can we please get some of that stuff updated, or at least a
> clearer explanation of what's going on with $extension_schema here?
>
I've added these changes to restrict users from explicitly setting the
$extension_schema in the search_path. This ensures that
$extension_schema can only be set implicitly for functions created by
the extension when the "protected" flag is enabled.
I apologize for not commenting on this change initially. I'll review
the patch, add comments where needed, and submit an updated version.
--
With Regards,
Ashutosh Sharma.
