Dear Amit,
Thanks for the comment! PSA new version.
>
> Few comments:
> 1. Why do we need to invalidate relsync entries when owner of its
> publication changes?
>
> I think the owner change will impact the future Alter Publication ...
> Add/Drop/Set/Rename operations as that will be allowed only to new
> owner (or super users), otherwise, there shouldn't be an impact on
> RelSyncCache entries. Am, I missing something?
Actually, I did not find reasons to invalidate even OWNER command for now. I
included it to 1) follow current rule, and to 2) prepare for future update.
1) - Currently RelSyncCache entries are discarded even by ALTER PUBLICATION OWNER
statements. I wanted to preserve it.
2) - In future versions, privilege might be introduced for the publications,
some publications would not be visible for other db users. In this case
I feel we should modify RelationSyncEntry::pubactions, columns and
exprstate thus entries must be discarded.
Now I removed invalidations for OWNER command. Let's revert the change if we miss
something.
> 2.
> + if (pubform->puballtables)
> + {
> + CacheInvalidateRelSyncAll();
> + }
> + else
> + {
> + List *relids = NIL;
> + List *schemarelids = NIL;
> +
> + /*
> + * For partition table, when we insert data, get_rel_sync_entry is
> + * called and a hash entry is created for the corresponding leaf table.
> + * So invalidating the leaf nodes would be sufficient here.
> + */
> + relids = GetPublicationRelations(pubform->oid,
> + PUBLICATION_PART_LEAF);
> + schemarelids = GetAllSchemaPublicationRelations(pubform->oid,
> + PUBLICATION_PART_LEAF);
> +
> + relids = list_concat_unique_oid(relids, schemarelids);
> +
> + InvalidateRelSyncCaches(relids);
> + }
> +
> + CatalogTupleUpdate(rel, &tup->t_self, tup);
>
> Shouldn't we need to update the CatalogTuple before invalidations.
Right, fixed.
> 3.
> + if (pubform->puballtables)
> + {
> + CacheInvalidateRelSyncAll();
> + }
> + else
> + {
> + List *relids = NIL;
> + List *schemarelids = NIL;
> +
> + /*
> + * For partition table, when we insert data, get_rel_sync_entry is
> + * called and a hash entry is created for the corresponding leaf table.
> + * So invalidating the leaf nodes would be sufficient here.
> + */
> + relids = GetPublicationRelations(pubform->oid,
> + PUBLICATION_PART_LEAF);
> + schemarelids = GetAllSchemaPublicationRelations(pubform->oid,
> + PUBLICATION_PART_LEAF);
> +
> + relids = list_concat_unique_oid(relids, schemarelids);
> +
> + InvalidateRelSyncCaches(relids);
> + }
>
> This is a duplicate code. Can we write a function to eliminate this duplicacy?
Since the part has been removed from OWNER command, duplicacy was removed.
I did not introduce a function for this.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
