Re: Added schema level support for publication. - Mailing list pgsql-hackers

On Thu, Dec 19, 2024 at 12:02 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 18 Dec 2024 at 16:34, Artur Zakirov <zaartur@gmail.com> wrote:
> >
> > On Tue, 17 Dec 2024 at 10:43, vignesh C <vignesh21@gmail.com> wrote:
> > > > If I understand your suggestion correctly I think this will break the
> > > > "--exclude-schema" option of pg_dump. That change will dump all
> > > > mappings between publications and schemas for publications which are
> > > > dumped.
> > > >
> > > > That solves the issue with special schemas, but restore will fail if
> > > > some schemas were explicitly excluded. pg_dump will include in the
> > > > dump ALTER PUBLICATION <pub> ADD TABLES IN SCHEMA <schema> even for
> > > > those schemas which are not created during restore.
> > >
> > > This is already the case in the existing implementation, so users
> > > should not be surprised by the proposed change.
> >
> > Currently the behavior isn't the same as the proposed change.
> >
> > Sorry, I might have been not clear when I described what might be
> > wrong with this. Here is the example with the proposed patch [1].
> >
> > Create necessary objects to test:
> >
> >     create schema nsp;
> >     create publication pub for tables in schema nsp;
> >
> > If you run pg_dump excluding the schema "nsp":
> >
> >     pg_dump -d postgres -U postgres -f backup --exclude-schema=nsp
> >
> > In the resulting file "backup" you will have:
> >
> >     ...
> >     ALTER PUBLICATION pub ADD TABLES IN SCHEMA nsp;
> >     ...
> >
> > which you won't have on the current master. And I think this is not
> > what users might expect and it can break some of the scenarios because
> > during restore they will have an error:
> >
> >     ERROR:  schema "nsp" does not exist
>
> Yes, this is done intentionally in the proposed patch to keep it
> consistent with other scenarios in HEAD.
> For example, consider the following case:
> -- Create schema and user defined function in schema sch2
> create schema sch2;
> CREATE FUNCTION sch2.add1(integer, integer)
> RETURNS integer
> LANGUAGE sql IMMUTABLE STRICT
> AS $_$select $1 + $2;$_$;
>
> -- Create a view which references user defined function of a different schema
> create schema sch1;
> CREATE TABLE sch1.t1 (c1 integer, c2 integer);
> CREATE VIEW sch1.v1 AS SELECT c1 FROM sch1.t1 WHERE (sch2.add1(c1, c2) >= 10);
>
> -- Exclude schema sch2 which has the user defined function while dumping
> ./pg_dump -d postgres -Fc -f dump1 -N sch2
>
> The dump file has the reference to sch2.add1 even though sch2 schema
> was excluded, dump will not have the user defined functions defined in
> schema sch2:
> CREATE VIEW sch1.v1 AS
>  SELECT c1
>    FROM sch1.t1
>   WHERE (sch2.add1(c1, c2) >= 10);
>
> Restore using the above dump that was generated will fail with the below error:
> ./pg_restore -d test1 dump1
> pg_restore: error: could not execute query: ERROR:  schema "sch2" does not exist
> LINE 4:   WHERE (sch2.add1(c1, c2) >= 10);
>                  ^
> Command was: CREATE VIEW sch1.v1 AS
>  SELECT c1
>    FROM sch1.t1
>   WHERE (sch2.add1(c1, c2) >= 10);
>
> The proposed patch is in similar lines.
>

I agree with the proposed patch and this behavior. It is on the lines
of what Tom proposed as one of the ways to address the issue raised in
his initial email. Also, it follows the existing behavior in cases
like view<->function dependency as shown by you in this email, and to
some extent subscription<->publication dependency as shown in email
[1].

Let's wait till the beginning of the next CF to see if there are other
suggestions or any arguments against this proposed change.

[1] - https://www.postgresql.org/message-id/CAA4eK1%2BZYanA51c9NzKM31AqJSw-j0-edGz91%2BVh-nsoKdzKfQ%40mail.gmail.com
--
With Regards,
Amit Kapila.