At Thu, 14 Mar 2019 11:30:12 +0900, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote in
<59e5a734-9e06-1035-385b-626717581...@lab.ntt.co.jp>
> On 2019/03/13 21:03, Peter Eisentraut wrote:
> > If a FOR ALL TABLES publication exists, unlogged tables are ignored
> > for publishing changes. But CheckCmdReplicaIdentity() would still
> > check in that case that such a table has a replica identity set before
> > accepting updates. That is useless, so check first whether the given
> > table is publishable and skip the check if not.
> >
> > Example:
> >
> > CREATE PUBLICATION pub FOR ALL TABLES;
> > CREATE UNLOGGED TABLE logical_replication_test AS SELECT 1 AS number;
> > UPDATE logical_replication_test SET number = 2;
> > ERROR: cannot update table "logical_replication_test" because it does
> > not have a replica identity and publishes updates
> >
> > Patch attached.
>
> An email on -bugs earlier this morning complains of the same problem but
> for temporary tables.
>
> https://www.postgresql.org/message-id/CAHOFxGr%3DmqPZXbAuoR7Nbq-bU4HxqVWHbTTUy5%3DPKQut_F0%3DXA%40mail.gmail.com
>
> It seems your patch fixes their case too.
Is it the right thing that GetRelationPublicationsActions sets
wrong rd_publicatons for the relations?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 2704c5fbb65ebfee144a37c6b34eccdd853033a0 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp>
Date: Thu, 14 Mar 2019 15:02:20 +0900
Subject: [PATCH 1/2] step1
---
src/backend/utils/cache/relcache.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index eba77491fd..a43fb040cb 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5089,6 +5089,8 @@ GetRelationPublicationActions(Relation relation)
return memcpy(pubactions, relation->rd_pubactions,
sizeof(PublicationActions));
+ if (is_publishable_relation(relation))
+ {
/* Fetch the publication membership info. */
puboids = GetRelationPublications(RelationGetRelid(relation));
puboids = list_concat_unique_oid(puboids, GetAllTablesPublications());
@@ -5121,6 +5123,7 @@ GetRelationPublicationActions(Relation relation)
pubactions->pubdelete && pubactions->pubtruncate)
break;
}
+ }
if (relation->rd_pubactions)
{
--
2.16.3
>From b0946c5b97857cf476975306ce78355f9316e722 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp>
Date: Thu, 14 Mar 2019 15:02:54 +0900
Subject: [PATCH 2/2] step2: fix indentation
---
src/backend/utils/cache/relcache.c | 50 +++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index a43fb040cb..f5dff5572e 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5091,38 +5091,38 @@ GetRelationPublicationActions(Relation relation)
if (is_publishable_relation(relation))
{
- /* Fetch the publication membership info. */
- puboids = GetRelationPublications(RelationGetRelid(relation));
- puboids = list_concat_unique_oid(puboids, GetAllTablesPublications());
+ /* Fetch the publication membership info. */
+ puboids = GetRelationPublications(RelationGetRelid(relation));
+ puboids = list_concat_unique_oid(puboids, GetAllTablesPublications());
- foreach(lc, puboids)
- {
- Oid pubid = lfirst_oid(lc);
- HeapTuple tup;
- Form_pg_publication pubform;
+ foreach(lc, puboids)
+ {
+ Oid pubid = lfirst_oid(lc);
+ HeapTuple tup;
+ Form_pg_publication pubform;
- tup = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubid));
+ tup = SearchSysCache1(PUBLICATIONOID, ObjectIdGetDatum(pubid));
- if (!HeapTupleIsValid(tup))
- elog(ERROR, "cache lookup failed for publication %u", pubid);
+ if (!HeapTupleIsValid(tup))
+ elog(ERROR, "cache lookup failed for publication %u", pubid);
- pubform = (Form_pg_publication) GETSTRUCT(tup);
+ pubform = (Form_pg_publication) GETSTRUCT(tup);
- pubactions->pubinsert |= pubform->pubinsert;
- pubactions->pubupdate |= pubform->pubupdate;
- pubactions->pubdelete |= pubform->pubdelete;
- pubactions->pubtruncate |= pubform->pubtruncate;
+ pubactions->pubinsert |= pubform->pubinsert;
+ pubactions->pubupdate |= pubform->pubupdate;
+ pubactions->pubdelete |= pubform->pubdelete;
+ pubactions->pubtruncate |= pubform->pubtruncate;
- ReleaseSysCache(tup);
+ ReleaseSysCache(tup);
- /*
- * If we know everything is replicated, there is no point to check for
- * other publications.
- */
- if (pubactions->pubinsert && pubactions->pubupdate &&
- pubactions->pubdelete && pubactions->pubtruncate)
- break;
- }
+ /*
+ * If we know everything is replicated, there is no point to check
+ * for other publications.
+ */
+ if (pubactions->pubinsert && pubactions->pubupdate &&
+ pubactions->pubdelete && pubactions->pubtruncate)
+ break;
+ }
}
if (relation->rd_pubactions)
--
2.16.3
