Re: Added schema level support for publication.

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Ajin Cherian <itsajin@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>
Date: 2021-07-12T09:20:38Z
Lists: pgsql-hackers

Attachments

On Fri, Jul 9, 2021 at 8:58 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Thursday, July 8, 2021 11:47 AM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote
> > On Wednesday, June 30, 2021 7:43 PM vignesh C <vignesh21@gmail.com>
> > wrote:
> > > Thanks for reporting this issue, the attached v9 patch fixes this issue. This also
> > fixes the other issue you reported at [1].
> >
> > Hi,
> >
> > I had a look at the patch, please consider following comments.
>
> Some more commets.
>
> Currently, postgres caches publication actions info in the
> RelationData::rd_pubactions, but after applying the patch, it seems
> rd_pubactions is not initialized when using schema level publication.
>
> It cound result in some unexpected behaviour when checking if command can be
> executed with current replica identity.
>
> ----
> CheckCmdReplicaIdentity
> ...
>         pubactions = GetRelationPublicationActions(rel);
>         if (cmd == CMD_UPDATE && pubactions->pubupdate)
>                 ereport(ERROR,
>                                 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>                                  errmsg("cannot update table \"%s\" because it does not have a replica identity and publishes updates",
>                                                 RelationGetRelationName(rel)),
>                                  errhint("To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.")));
> ----
> Based on the above code, we access rd_pubactions from relcache in function
> GetRelationPublicationActions(), if we don't have correct publication
> information, we won't get the error in publication, instead we could lead to
> the failure in subscription side.
>
> To fix, I think (1) the patch can modify the function
> GetRelationPublicationActions() to get the schema level publication related to
> the relation, then merge the publication action to the rd_pubactions.
>
> In addition, (2) it seems we also need to add the relcache invalidation code
> about schema level publication when alter publication options.
>
> I attached a rough code diff about (1) and (2), the diff is based on the v9 patchset.
> I hope it can help fix the above issues.

Thanks for identifying the issues and also providing the fix for it.
Those changes are required, I have taken your changes into my patch.
The Attached patch has the changes for the same.

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.