Re: Support logical replication of DDLs

On Sun, Feb 19, 2023 at 7:50 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>
> On 2/17/23 4:15 AM, Amit Kapila wrote:
>
> > This happens because of the function used in the index expression.
> > Now, this is not the only thing, the replication can even fail during
> > DDL replication when the function like above is IMMUTABLE and used as
> > follows: ALTER TABLE tbl ADD COLUMN d int DEFAULT t(1);
> >
> > Normally, it is recommended that users can fix such errors by
> > schema-qualifying affected names. See commits 11da97024a and
> > 582edc369c.
>
> I'm very familiar with those CVEs, but even though these are our
> recommended best practices, there is still a lot of code that does not
> schema-qualify the names of functions (including many of our own examples ;)
>
> If we're going to say "You can use logical replication to replicate
> functions, but you have to ensure you've schema-qualified any function
> calls within them," I think that will prevent people from being able to
> use this feature, particularly on existing applications.
>

I agree with this statement.

> I guess there's a connection I'm missing here. For the failing examples
> above, I look at the pg_proc entries on both the publisher and the
> subscriber and they're identical. I'm not understanding why creating and
> executing the functions works on the publisher, but it does not on the
> subscriber.
>

It is because on the subscriber, in apply worker, we override the
search path to "". See

InitializeApplyWorker()
{
...
/*
* Set always-secure search path, so malicious users can't redirect user
* code (e.g. pg_index.indexprs).
*/
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
...

This has been done to ensure that apply worker doesn't execute
arbitrary expressions as currently, it works with the privileges of
the subscription owner which would be a superuser.

> What additional info would the subscriber need to be able to
> successfully run these functions? Would we need to pass in some
> additional context, e.g. what the search_path was at the time the
> publisher created the function?
>

Yeah, I think search_path is required. I think we need some way to
avoid breaking what we have done in commit 11da97024a and that also
allows us to refer to objects without schema qualification in
functions. Will it be sane to allow specifying search_path for a
subscription via Alter Subscription? Can we think of any other way
here?

-- 
With Regards,
Amit Kapila.