Re: Added schema level support for publication.

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Greg Nancarrow <gregn4422@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Smith <smithpb2250@gmail.com>, Ajin Cherian <itsajin@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-10-11T07:05:28Z
Lists: pgsql-hackers
On Mon, Oct 11, 2021 at 7:46 AM tanghy.fnst@fujitsu.com
<tanghy.fnst@fujitsu.com> wrote:
>
> > On Friday, October 8, 2021 7:05 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > v37-0003-Tests-for-FOR-ALL-TABLES-IN-SCHEMA-publication
> > 3.
> > --- a/src/bin/pg_dump/t/002_pg_dump.pl
> > +++ b/src/bin/pg_dump/t/002_pg_dump.pl
> > ..
> > + 'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test' => {
> > + create_order => 51,
> > + create_sql =>
> > +   'ALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test;',
> > + regexp => qr/^
> > + \QALTER PUBLICATION pub3 ADD ALL TABLES IN SCHEMA dump_test;\E
> > + /xm,
> > + like   => { %full_runs, section_post_data => 1, },
> > + unlike => { exclude_dump_test_schema => 1, },
> >
> > In this test, won't it exclude the schema dump_test because of unlike?
> > If so, then we don't have coverage for "ALL Tables In Schema" except
> > for public schema?
> >
>
> Yes, the unlike case will exclude the schema dump_test, but I think schema dump_test could be
> dumped in like case.
> I checked the log file src/bin/pg_dump/tmp_check/log/regress_log_002_pg_dump and
> saw some cases were described as "should dump ALTER PUBLICATION pub3 ADD ALL
> TABLES IN SCHEMA dump_test". I think in these cases schema dump_test would be
> dumped.

I agree

> Besides, a small comment on tab-complete.c:
>
>         else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL"))
> -               COMPLETE_WITH("TABLES");
> -       else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES")
> -                        || Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLE", MatchAny))
> +               COMPLETE_WITH("TABLES", "TABLE IN SCHEMA");
>
>
> COMPLETE_WITH("TABLES", "TABLE IN SCHEMA");
> ->
> COMPLETE_WITH("TABLES", "TABLES IN SCHEMA");

Modified.
This issue is fixed in the v38 patch attached at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm1TP9S0dif2QWoEUcCtNDop1xJ6Rj1xnu2vS92%3Dj9ahYw%40mail.gmail.com

Regards,
Vignesh



Commits

  1. Include schema/table publications even with exclude options in dump.

  2. Rename some enums to use TABLE instead of REL.

  3. Add tap tests for the schema publications.

  4. Allow publishing the tables of schema.

  5. In pg_dump, use simplehash.h to look up dumpable objects by OID.