From 50e0570fe2c83dfddc85f3e7df0ffebb12c5ef3e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sat, 13 Nov 2021 02:33:44 +0000
Subject: [PATCH v10] Improve publication error messages

Provide accurate messages for CREATE PUBLICATION commands. Also add
tests to cover temporary, unlogged, system and foreign tables.

While at it, modify errdetail_relkind_not_supported API to distinguish
tables based on persistence property and report for system tables.
Also, introduce errdetail_relkind_not_supported_v2 so that the callers
not having Form_pg_class object can send relid, relkind, relpersistence
as direct parameters. This way, the API is more flexbile and easier
to use. It also provides accurate detail messages for future work.
---
 contrib/amcheck/verify_heapam.c               |  2 +-
 contrib/pageinspect/rawpage.c                 |  2 +-
 contrib/pg_surgery/heap_surgery.c             |  2 +-
 contrib/pg_visibility/pg_visibility.c         |  2 +-
 contrib/pgstattuple/pgstatapprox.c            |  2 +-
 contrib/pgstattuple/pgstatindex.c             |  2 +-
 contrib/pgstattuple/pgstattuple.c             |  2 +-
 .../postgres_fdw/expected/postgres_fdw.out    |  6 ++++
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  5 ++++
 src/backend/catalog/pg_class.c                | 24 ++++++++++++++--
 src/backend/catalog/pg_publication.c          | 28 ++++++-------------
 src/backend/commands/comment.c                |  2 +-
 src/backend/commands/indexcmds.c              |  2 +-
 src/backend/commands/lockcmds.c               |  5 ++--
 src/backend/commands/seclabel.c               |  2 +-
 src/backend/commands/sequence.c               |  2 +-
 src/backend/commands/statscmds.c              |  2 +-
 src/backend/commands/subscriptioncmds.c       |  4 +--
 src/backend/commands/tablecmds.c              |  9 +++---
 src/backend/commands/trigger.c                |  6 ++--
 src/backend/executor/execReplication.c        |  5 ++--
 src/backend/parser/parse_utilcmd.c            |  2 +-
 src/backend/replication/logical/relation.c    |  2 +-
 src/backend/rewrite/rewriteDefine.c           |  4 +--
 src/include/catalog/pg_class.h                |  4 ++-
 src/include/executor/executor.h               |  2 +-
 src/test/regress/expected/publication.out     | 16 +++++++++++
 src/test/regress/sql/publication.sql          | 14 ++++++++++
 28 files changed, 107 insertions(+), 53 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index bae5340111..f6b16c8fb0 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -314,7 +314,7 @@ verify_heapam(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot check relation \"%s\"",
 						RelationGetRelationName(ctx.rel)),
-				 errdetail_relkind_not_supported(ctx.rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(ctx.rel), ctx.rel->rd_rel)));
 
 	/*
 	 * Sequences always use heap AM, but they don't show that in the catalogs.
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 4bfa346c24..3173fb83d3 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -160,7 +160,7 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot get raw page from relation \"%s\"",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	/*
 	 * Reject attempts to read non-local temporary relations; we would be
diff --git a/contrib/pg_surgery/heap_surgery.c b/contrib/pg_surgery/heap_surgery.c
index 7edfe4f326..043efda2f6 100644
--- a/contrib/pg_surgery/heap_surgery.c
+++ b/contrib/pg_surgery/heap_surgery.c
@@ -110,7 +110,7 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot operate on relation \"%s\"",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
 		ereport(ERROR,
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index b5362edcee..b180174a1c 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -783,5 +783,5 @@ check_relation_relkind(Relation rel)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" is of wrong relation kind",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 }
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 3b836f370e..d824d90351 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -291,7 +291,7 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("relation \"%s\" is of wrong relation kind",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
 		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 6c4b053dd0..45af621e95 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -450,7 +450,7 @@ pg_relpages_impl(Relation rel)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot get page count of relation \"%s\"",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	/* note: this will work OK on non-local temp tables */
 
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index f408e6d84d..bacaaa3b9e 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -295,7 +295,7 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo)
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("cannot get tuple-level statistics for relation \"%s\"",
 							RelationGetRelationName(rel)),
-					 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 	}
 
 	return 0;					/* should not happen */
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 3cee0a8c12..786781db4b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -256,6 +256,12 @@ SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1;  -- should work again
 ANALYZE ft1;
 ALTER FOREIGN TABLE ft2 OPTIONS (use_remote_estimate 'true');
 -- ===================================================================
+-- test error case for create publication on foreign table
+-- ===================================================================
+CREATE PUBLICATION testpub_ftbl FOR TABLE ft1;  -- should fail
+ERROR:  cannot add relation "ft1" to publication
+DETAIL:  This operation is not supported for foreign tables.
+-- ===================================================================
 -- simple queries
 -- ===================================================================
 -- single table without alias
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index e40112e41d..666d21962a 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -247,6 +247,11 @@ SELECT c3, c4 FROM ft1 ORDER BY c3, c1 LIMIT 1;  -- should work again
 ANALYZE ft1;
 ALTER FOREIGN TABLE ft2 OPTIONS (use_remote_estimate 'true');
 
+-- ===================================================================
+-- test error case for create publication on foreign table
+-- ===================================================================
+CREATE PUBLICATION testpub_ftbl FOR TABLE ft1;  -- should fail
+
 -- ===================================================================
 -- simple queries
 -- ===================================================================
diff --git a/src/backend/catalog/pg_class.c b/src/backend/catalog/pg_class.c
index 304c0af808..e713c6bdb2 100644
--- a/src/backend/catalog/pg_class.c
+++ b/src/backend/catalog/pg_class.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "catalog/catalog.h"
 #include "catalog/pg_class.h"
 
 /*
@@ -21,12 +22,31 @@
  * operation.
  */
 int
-errdetail_relkind_not_supported(char relkind)
+errdetail_relkind_not_supported(Oid relid, Form_pg_class rd_rel)
+{
+	return errdetail_relkind_not_supported_v2(relid, rd_rel->relkind,
+											  rd_rel->relpersistence);
+}
+
+/*
+ * Same as errdetail_relkind_not_supported, but the input parameters are passed
+ * explicitly.
+ */
+int
+errdetail_relkind_not_supported_v2(Oid relid, char relkind, char relpersistence)
 {
 	switch (relkind)
 	{
 		case RELKIND_RELATION:
-			return errdetail("This operation is not supported for tables.");
+			if (OidIsValid(relid) && IsCatalogRelationOid(relid))
+				return errdetail("This operation is not supported for system tables.");
+			else if (relpersistence == RELPERSISTENCE_PERMANENT)
+				return errdetail("This operation is not supported for tables.");
+			else if (relpersistence == RELPERSISTENCE_UNLOGGED)
+				return errdetail("This operation is not supported for unlogged tables.");
+			else if (relpersistence == RELPERSISTENCE_TEMP)
+				return errdetail("This operation is not supported for temporary tables.");
+			break;
 		case RELKIND_INDEX:
 			return errdetail("This operation is not supported for indexes.");
 		case RELKIND_SEQUENCE:
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index fed83b89a9..82c405f245 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -52,30 +52,18 @@
 static void
 check_publication_add_relation(Relation targetrel)
 {
-	/* Must be a regular or partitioned table */
-	if (RelationGetForm(targetrel)->relkind != RELKIND_RELATION &&
-		RelationGetForm(targetrel)->relkind != RELKIND_PARTITIONED_TABLE)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cannot add relation \"%s\" to publication",
-						RelationGetRelationName(targetrel)),
-				 errdetail_relkind_not_supported(RelationGetForm(targetrel)->relkind)));
-
-	/* Can't be system table */
-	if (IsCatalogRelation(targetrel))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cannot add relation \"%s\" to publication",
-						RelationGetRelationName(targetrel)),
-				 errdetail("This operation is not supported for system tables.")));
-
-	/* UNLOGGED and TEMP relations cannot be part of publication. */
-	if (!RelationIsPermanent(targetrel))
+	/*
+	 * Must be a regular or partitioned table. System table, UNLOGGED and TEMP
+	 * table cannot be part of publication.
+	 */
+	if ((RelationGetForm(targetrel)->relkind != RELKIND_RELATION &&
+		RelationGetForm(targetrel)->relkind != RELKIND_PARTITIONED_TABLE) ||
+		(IsCatalogRelation(targetrel) || !RelationIsPermanent(targetrel)))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("cannot add relation \"%s\" to publication",
 						RelationGetRelationName(targetrel)),
-				 errdetail("Temporary and unlogged relations cannot be replicated.")));
+				 errdetail_relkind_not_supported(RelationGetRelid(targetrel), RelationGetForm(targetrel))));
 }
 
 /*
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index d4943e374a..a93c1a304b 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -100,7 +100,7 @@ CommentObject(CommentStmt *stmt)
 						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 						 errmsg("cannot set comment on relation \"%s\"",
 								RelationGetRelationName(relation)),
-						 errdetail_relkind_not_supported(relation->rd_rel->relkind)));
+						 errdetail_relkind_not_supported(RelationGetRelid(relation), relation->rd_rel)));
 			break;
 		default:
 			break;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index c14ca27c5e..0e518f2b71 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -655,7 +655,7 @@ DefineIndex(Oid relationId,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("cannot create index on relation \"%s\"",
 							RelationGetRelationName(rel)),
-					 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 			break;
 	}
 
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 62465bacd8..38d97f10a6 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -84,6 +84,8 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
 		return;					/* woops, concurrently dropped; no permissions
 								 * check */
 
+	relpersistence = get_rel_persistence(relid);
+
 	/* Currently, we only allow plain tables or views to be locked */
 	if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE &&
 		relkind != RELKIND_VIEW)
@@ -91,13 +93,12 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot lock relation \"%s\"",
 						rv->relname),
-				 errdetail_relkind_not_supported(relkind)));
+				 errdetail_relkind_not_supported_v2(relid, relkind, relpersistence)));
 
 	/*
 	 * Make note if a temporary relation has been accessed in this
 	 * transaction.
 	 */
-	relpersistence = get_rel_persistence(relid);
 	if (relpersistence == RELPERSISTENCE_TEMP)
 		MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
 
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index 53c18628a7..9437b0ca50 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -191,7 +191,7 @@ ExecSecLabelStmt(SecLabelStmt *stmt)
 						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 						 errmsg("cannot set security label on relation \"%s\"",
 								RelationGetRelationName(relation)),
-						 errdetail_relkind_not_supported(relation->rd_rel->relkind)));
+						 errdetail_relkind_not_supported(RelationGetRelid(relation), relation->rd_rel)));
 			break;
 		default:
 			break;
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 72bfdc07a4..dfac3678a8 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1655,7 +1655,7 @@ process_owned_by(Relation seqrel, List *owned_by, bool for_identity)
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("sequence cannot be owned by relation \"%s\"",
 							RelationGetRelationName(tablerel)),
-					 errdetail_relkind_not_supported(tablerel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(tablerel), tablerel->rd_rel)));
 
 		/* We insist on same owner and schema */
 		if (seqrel->rd_rel->relowner != tablerel->rd_rel->relowner)
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 8f1550ec80..2ad1f8d057 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -138,7 +138,7 @@ CreateStatistics(CreateStatsStmt *stmt)
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("cannot define statistics for relation \"%s\"",
 							RelationGetRelationName(rel)),
-					 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 		/* You must own the relation to create stats on it */
 		if (!pg_class_ownercheck(RelationGetRelid(rel), stxowner))
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index c47ba26369..bbd250e012 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -533,7 +533,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
 				relid = RangeVarGetRelid(rv, AccessShareLock, false);
 
 				/* Check for supported relkind. */
-				CheckSubscriptionRelkind(get_rel_relkind(relid),
+				CheckSubscriptionRelkind(relid, get_rel_relkind(relid),
 										 rv->schemaname, rv->relname);
 
 				AddSubscriptionRelState(subid, relid, table_state,
@@ -683,7 +683,7 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data)
 			relid = RangeVarGetRelid(rv, AccessShareLock, false);
 
 			/* Check for supported relkind. */
-			CheckSubscriptionRelkind(get_rel_relkind(relid),
+			CheckSubscriptionRelkind(relid, get_rel_relkind(relid),
 									 rv->schemaname, rv->relname);
 
 			pubrel_local_oids[off++] = relid;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 857cc5ce6e..1763a24680 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3327,6 +3327,7 @@ static void
 renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing)
 {
 	char		relkind = classform->relkind;
+	char		relpersistence = classform->relpersistence;
 
 	if (classform->reloftype && !recursing)
 		ereport(ERROR,
@@ -3352,7 +3353,7 @@ renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot rename columns of relation \"%s\"",
 						NameStr(classform->relname)),
-				 errdetail_relkind_not_supported(relkind)));
+				 errdetail_relkind_not_supported_v2(myrelid, relkind, relpersistence)));
 
 	/*
 	 * permissions checking.  only the owner of a class can change its schema.
@@ -6192,7 +6193,7 @@ ATSimplePermissions(AlterTableType cmdtype, Relation rel, int allowed_targets)
 			/* translator: %s is a group of some SQL keywords */
 					 errmsg("ALTER action %s cannot be performed on relation \"%s\"",
 							action_str, RelationGetRelationName(rel)),
-					 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 		else
 			/* internal error? */
 			elog(ERROR, "invalid ALTER action attempted on relation \"%s\"",
@@ -13357,7 +13358,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("cannot change owner of relation \"%s\"",
 							NameStr(tuple_class->relname)),
-					 errdetail_relkind_not_supported(tuple_class->relkind)));
+					 errdetail_relkind_not_supported(relationOid, tuple_class)));
 	}
 
 	/*
@@ -13796,7 +13797,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 					 errmsg("cannot set options for relation \"%s\"",
 							RelationGetRelationName(rel)),
-					 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+					 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 			break;
 	}
 
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d8890d2c74..9aa8aa2e13 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -312,7 +312,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" cannot have triggers",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	if (!allowSystemTableMods && IsSystemRelation(rel))
 		ereport(ERROR,
@@ -1289,7 +1289,7 @@ RemoveTriggerById(Oid trigOid)
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" cannot have triggers",
 						RelationGetRelationName(rel)),
-				 errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(rel), rel->rd_rel)));
 
 	if (!allowSystemTableMods && IsSystemRelation(rel))
 		ereport(ERROR,
@@ -1396,7 +1396,7 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" cannot have triggers",
 						rv->relname),
-				 errdetail_relkind_not_supported(form->relkind)));
+				 errdetail_relkind_not_supported(relid, form)));
 
 	/* you must own the table to rename one of its triggers */
 	if (!pg_class_ownercheck(relid, GetUserId()))
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 574d7d27fd..f8bcb98e44 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -605,7 +605,8 @@ CheckCmdReplicaIdentity(Relation rel, CmdType cmd)
  * The nspname and relname are only needed for error reporting.
  */
 void
-CheckSubscriptionRelkind(char relkind, const char *nspname,
+CheckSubscriptionRelkind(Oid relid, char relkind,
+						 const char *nspname,
 						 const char *relname)
 {
 	if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
@@ -613,5 +614,5 @@ CheckSubscriptionRelkind(char relkind, const char *nspname,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("cannot use relation \"%s.%s\" as logical replication target",
 						nspname, relname),
-				 errdetail_relkind_not_supported(relkind)));
+				 errdetail_relkind_not_supported_v2(relid, relkind, RELPERSISTENCE_PERMANENT)));
 }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 313d7b6ff0..8b9ef9e1f8 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -976,7 +976,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" is invalid in LIKE clause",
 						RelationGetRelationName(relation)),
-				 errdetail_relkind_not_supported(relation->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(relation), relation->rd_rel)));
 
 	cancel_parser_errposition_callback(&pcbstate);
 
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index c37e2a7e29..ab174a48da 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -326,7 +326,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 		entry->localreloid = relid;
 
 		/* Check for supported relkind. */
-		CheckSubscriptionRelkind(entry->localrel->rd_rel->relkind,
+		CheckSubscriptionRelkind(relid, entry->localrel->rd_rel->relkind,
 								 remoterel->nspname, remoterel->relname);
 
 		/*
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 6589345ac4..0ffb77a0cb 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -270,7 +270,7 @@ DefineQueryRewrite(const char *rulename,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" cannot have rules",
 						RelationGetRelationName(event_relation)),
-				 errdetail_relkind_not_supported(event_relation->rd_rel->relkind)));
+				 errdetail_relkind_not_supported(RelationGetRelid(event_relation), event_relation->rd_rel)));
 
 	if (!allowSystemTableMods && IsSystemRelation(event_relation))
 		ereport(ERROR,
@@ -937,7 +937,7 @@ RangeVarCallbackForRenameRule(const RangeVar *rv, Oid relid, Oid oldrelid,
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("relation \"%s\" cannot have rules", rv->relname),
-				 errdetail_relkind_not_supported(form->relkind)));
+				 errdetail_relkind_not_supported(relid, form)));
 
 	if (!allowSystemTableMods && IsSystemClass(relid, form))
 		ereport(ERROR,
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index fef9945ed8..aa95a2004a 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -156,6 +156,8 @@ DECLARE_UNIQUE_INDEX_PKEY(pg_class_oid_index, 2662, ClassOidIndexId, on pg_class
 DECLARE_UNIQUE_INDEX(pg_class_relname_nsp_index, 2663, ClassNameNspIndexId, on pg_class using btree(relname name_ops, relnamespace oid_ops));
 DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, ClassTblspcRelfilenodeIndexId, on pg_class using btree(reltablespace oid_ops, relfilenode oid_ops));
 
+extern int errdetail_relkind_not_supported(Oid relid, Form_pg_class rd_rel);
+
 #ifdef EXPOSE_TO_CLIENT_CODE
 
 #define		  RELKIND_RELATION		  'r'	/* ordinary table */
@@ -198,7 +200,7 @@ DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, ClassTblspcRelfilenodeInd
 	 (relkind) == RELKIND_TOASTVALUE || \
 	 (relkind) == RELKIND_MATVIEW)
 
-extern int errdetail_relkind_not_supported(char relkind);
+extern int errdetail_relkind_not_supported_v2(Oid relid, char relkind, char relpersistence);
 
 #endif							/* EXPOSE_TO_CLIENT_CODE */
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index cd57a704ad..45582f896d 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -644,7 +644,7 @@ extern void ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
 									 TupleTableSlot *searchslot);
 extern void CheckCmdReplicaIdentity(Relation rel, CmdType cmd);
 
-extern void CheckSubscriptionRelkind(char relkind, const char *nspname,
+extern void CheckSubscriptionRelkind(Oid relid, char relkind, const char *nspname,
 									 const char *relname);
 
 /*
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 2ff21a7543..1feb558968 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -258,6 +258,22 @@ DROP TABLE testpub_tbl4;
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
 ERROR:  cannot add relation "testpub_view" to publication
 DETAIL:  This operation is not supported for views.
+CREATE TEMPORARY TABLE testpub_temptbl(a int);
+-- fail - temporary table
+CREATE PUBLICATION testpub_fortemptbl FOR TABLE testpub_temptbl;
+ERROR:  cannot add relation "testpub_temptbl" to publication
+DETAIL:  This operation is not supported for temporary tables.
+DROP TABLE testpub_temptbl;
+CREATE UNLOGGED TABLE testpub_unloggedtbl(a int);
+-- fail - unlogged table
+CREATE PUBLICATION testpub_forunloggedtbl FOR TABLE testpub_unloggedtbl;
+ERROR:  cannot add relation "testpub_unloggedtbl" to publication
+DETAIL:  This operation is not supported for unlogged tables.
+DROP TABLE testpub_unloggedtbl;
+-- fail - system table
+CREATE PUBLICATION testpub_forsystemtbl FOR TABLE pg_publication;
+ERROR:  cannot add relation "pg_publication" to publication
+DETAIL:  This operation is not supported for system tables.
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
 RESET client_min_messages;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 85a5302a74..8fa0435c32 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -150,6 +150,20 @@ DROP TABLE testpub_tbl4;
 
 -- fail - view
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
+
+CREATE TEMPORARY TABLE testpub_temptbl(a int);
+-- fail - temporary table
+CREATE PUBLICATION testpub_fortemptbl FOR TABLE testpub_temptbl;
+DROP TABLE testpub_temptbl;
+
+CREATE UNLOGGED TABLE testpub_unloggedtbl(a int);
+-- fail - unlogged table
+CREATE PUBLICATION testpub_forunloggedtbl FOR TABLE testpub_unloggedtbl;
+DROP TABLE testpub_unloggedtbl;
+
+-- fail - system table
+CREATE PUBLICATION testpub_forsystemtbl FOR TABLE pg_publication;
+
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
 RESET client_min_messages;
-- 
2.25.1

