Re: Fix pg_publication_tables to exclude generated columns

On Tue, Jan 10, 2023 at 8:38 AM shiy(dot)fnst(at)fujitsu(dot)com
<shiy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Mon, Jan 9, 2023 11:06 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> > > On Mon, Jan 9, 2023 at 5:29 PM shiy(dot)fnst(at)fujitsu(dot)com
> > > <shiy(dot)fnst(at)fujitsu(dot)com> wrote:
> > >> I think one way to fix it is to modify pg_publication_tables query to exclude
> > >> generated columns. But in this way, we need to bump catalog version when
> > fixing
> > >> it in back-branch. Another way is to modify function
> > >> pg_get_publication_tables()'s return value to contain all supported columns
> > if
> > >> no column list is specified, and we don't need to change system view.
> >
> > > That sounds like a reasonable approach to fix the issue.
> >
> > We could just not fix it in the back branches. I'd argue that this is
> > as much a definition change as a bug fix, so it doesn't really feel
> > like something to back-patch anyway.
> >
>
> If this is not fixed in back-branch, in some cases we will get an error when
> creating/refreshing subscription because we query pg_publication_tables in
> column list check.
>
> e.g.
>
> -- publisher
> CREATE TABLE test_mix_4 (a int PRIMARY KEY, b int, c int, d int GENERATED ALWAYS AS (a + 1) STORED);
> CREATE PUBLICATION pub_mix_7 FOR TABLE test_mix_4 (a, b, c);
> CREATE PUBLICATION pub_mix_8 FOR TABLE test_mix_4;
>
> -- subscriber
> CREATE TABLE test_mix_4 (a int PRIMARY KEY, b int, c int, d int);
>
> postgres=# CREATE SUBSCRIPTION sub1 CONNECTION 'port=5432' PUBLICATION pub_mix_7, pub_mix_8;
> ERROR: cannot use different column lists for table "public.test_mix_4" in different publications
>
> I think it might be better to fix it in back-branch. And if we fix it by
> modifying pg_get_publication_tables(), we don't need to bump catalog version in
> back-branch, I think this seems acceptable.
>

So, if we don't backpatch then it could lead to an error when it
shouldn't have which is clearly a bug. I think we should backpatch
this unless Tom or others are against it.

--
With Regards,
Amit Kapila.