On Sat, Nov 13, 2021 at 7:57 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Sat, Nov 13, 2021 at 7:16 PM Euler Taveira <eu...@eulerto.com> wrote:
> > Thanks. It is a good idea to use errdetail_relkind_not_supported. I
> > slightly modified the API to "int errdetail_relkind_not_supported(Oid
> > relid, Form_pg_class rd_rel);" to simplify things and increase the
> > usability of the function further. For instance, it can report the
> > specific error for the catalog tables as well. And, also added "int
> > errdetail_relkind_not_supported _v2(Oid relid, char relkind, char
> > relpersistence);" so that the callers not having Form_pg_class (there
> > are 3 callers exist) can pass the parameters directly.
> >
> > Do we really need 2 functions? I don't think so. Besides that, relid is
> > redundant since this information is available in the Form_pg_class
struct.
>
> Yeah. The relid is available in Form_pg_class.
>
> Firstly, I didn't quite like the function
> errdetail_relkind_not_supported name to be too long here and adding to
> it the 2 or 3 parameters, in many places we are crossing 80 char
> limit. Above these, a function with one parameter is always better
> than function with 3 parameters.
>
> Having two functions isn't a big deal at all, I think we have many
> functions like that in the core (although I'm not gonna spend time
> finding all those functions, I'm sure there will be such functions).
>
> I would still go with with 2 functions:
>
> int errdetail_relkind_not_supported(Form_pg_class rd_rel);
> int errdetail_relkind_not_supported_v2(Oid relid, char relkind, char
> relpersistence);
>
> > int errdetail_relkind_not_supported(Oid relid, Form_pg_class rd_rel);
> >
> > My suggestion is to keep only the 3 parameter function:
> >
> > int errdetail_relkind_not_supported(Oid relid, char relkind, char
relpersistence);
> >
> > Multiple functions that is just a wrapper for a central one is a good
idea for
> > backward compatibility. That's not the case here.
>
> Since we are modifying it on the master, I think it is okay to have 2
> functions given the code simplification advantages we get with
> errdetail_relkind_not_supported(Form_pg_class rd_rel).
>
> I would even think further to rename "errdetail_relkind_not_supported"
> and have the following, because we don't have to be always descriptive
> in the function names. The errdetail would tell the function is going
> to give some error detail.
>
> int errdetail_relkind(Form_pg_class rd_rel);
> int errdetail_relkind_v2(Oid relid, char relkind, char relpersistence);
>
> or
>
> int errdetail_rel(Form_pg_class rd_rel);
> int errdetail_rel_v2(Oid relid, char relkind, char relpersistence);
>
> I prefer the above among the three function names.
>
> Thoughts?
PSA v11 patch with 2 APIs with much simpler parameters and small function
names:
int errdetail_rel(Form_pg_class rd_rel);
int errdetail_rel_v2(Oid relid, char relkind, char relpersistence);
Please review it.
Regards,
Bharath Rupireddy.
From 16876bdee3dccf6ec5c2faeb93dd3c51b62dbc2e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sun, 14 Nov 2021 03:57:11 +0000
Subject: [PATCH v11] 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.
As the function name is too long rename it to errdetail_rel.
Also, introduce errdetail_rel 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 | 25 +++++++++++++++--
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 | 5 ++--
src/include/executor/executor.h | 2 +-
src/test/regress/expected/publication.out | 16 +++++++++++
src/test/regress/sql/publication.sql | 14 ++++++++++
28 files changed, 108 insertions(+), 54 deletions(-)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index bae5340111..a9c84913b0 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_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..bc6aa5799a 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_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..d38884b3fa 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_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..6d89dba47f 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_rel(rel->rd_rel)));
}
diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c
index 3b836f370e..eef70df775 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_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..6779850142 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_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..e1890ca4a4 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_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..b4931b636d 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,30 @@
* operation.
*/
int
-errdetail_relkind_not_supported(char relkind)
+errdetail_rel(Form_pg_class rd_rel)
+{
+ return errdetail_rel_v2(rd_rel->oid, rd_rel->relkind,
+ rd_rel->relpersistence);
+}
+
+/*
+ * Same as errdetail_rel, but the input parameters are passed explicitly.
+ */
+int
+errdetail_rel_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:
@@ -49,4 +68,6 @@ errdetail_relkind_not_supported(char relkind)
elog(ERROR, "unrecognized relkind: '%c'", relkind);
return 0;
}
+
+ return 0;
}
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index fed83b89a9..d2315e212b 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_rel(RelationGetForm(targetrel))));
}
/*
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index d4943e374a..783f46786b 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_rel(relation->rd_rel)));
break;
default:
break;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index c14ca27c5e..05fb76368f 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_rel(rel->rd_rel)));
break;
}
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 62465bacd8..0cfe592fb5 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_rel_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..e8722af3f5 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_rel(relation->rd_rel)));
break;
default:
break;
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 72bfdc07a4..9888abeb48 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_rel(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..8f94b4d215 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_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..5f3a023ba9 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_rel_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_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_rel(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_rel(rel->rd_rel)));
break;
}
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d8890d2c74..27a51a5fc0 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_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_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_rel(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..03b99a912d 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_rel_v2(relid, relkind, RELPERSISTENCE_PERMANENT)));
}
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 313d7b6ff0..2d3665188d 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_rel(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..98829e78db 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_rel(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_rel(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..aaeede4651 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -156,6 +156,9 @@ 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_rel(Form_pg_class rd_rel);
+extern int errdetail_rel_v2(Oid relid, char relkind, char relpersistence);
+
#ifdef EXPOSE_TO_CLIENT_CODE
#define RELKIND_RELATION 'r' /* ordinary table */
@@ -198,8 +201,6 @@ DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, ClassTblspcRelfilenodeInd
(relkind) == RELKIND_TOASTVALUE || \
(relkind) == RELKIND_MATVIEW)
-extern int errdetail_relkind_not_supported(char relkind);
-
#endif /* EXPOSE_TO_CLIENT_CODE */
#endif /* PG_CLASS_H */
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
