pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedBetter error message for unsupported replication cases
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2022-02-15 e16db75dd6 efae4401c4 17a8c4c3a2 Improve subscriber's error message for wrong publication relkind.
* Better error message for unsupported replication cases
@ 2022-02-14 22:12 Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2022-02-14 22:12 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
In [1] there's a complaint that if you try to logically replicate
a partitioned table from v13-or-later to v12-or-earlier, you get
"table XXX not found on publisher", which is pretty confusing
because the publisher certainly does have such a table. That
happens because fetch_remote_table_info is too aggressive about
filtering by relkind and doesn't see the relation at all.
c314c147c improved that, but it wasn't back-patched. I propose
putting the attached into v10-v12. Maybe the error message
could be bikeshedded ... is "non-table relation" terminology
that we use in user-facing messages?
regards, tom lane
[1] https://www.postgresql.org/message-id/CANhtRiamAgYt1A-Nh4%3DmU3E1UhG9XPgB%2BX6mW1DWqa93vUXW9A%40mail...
Attachments:
handle-unsupported-relkind-better.patch (text/x-diff, 1.9K) download
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 2db88dc41a..61244e08fa 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -646,9 +646,10 @@ fetch_remote_table_info(char *nspname, char *relname,
WalRcvExecResult *res;
StringInfoData cmd;
TupleTableSlot *slot;
- Oid tableRow[2] = {OIDOID, CHAROID};
+ Oid tableRow[3] = {OIDOID, CHAROID, CHAROID};
Oid attrRow[4] = {TEXTOID, OIDOID, INT4OID, BOOLOID};
bool isnull;
+ char relkind;
int natt;
lrel->nspname = nspname;
@@ -656,13 +657,12 @@ fetch_remote_table_info(char *nspname, char *relname,
/* First fetch Oid and replica identity. */
initStringInfo(&cmd);
- appendStringInfo(&cmd, "SELECT c.oid, c.relreplident"
+ appendStringInfo(&cmd, "SELECT c.oid, c.relreplident, c.relkind"
" FROM pg_catalog.pg_class c"
" INNER JOIN pg_catalog.pg_namespace n"
" ON (c.relnamespace = n.oid)"
" WHERE n.nspname = %s"
- " AND c.relname = %s"
- " AND c.relkind = 'r'",
+ " AND c.relname = %s",
quote_literal_cstr(nspname),
quote_literal_cstr(relname));
res = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data,
@@ -683,6 +683,19 @@ fetch_remote_table_info(char *nspname, char *relname,
Assert(!isnull);
lrel->replident = DatumGetChar(slot_getattr(slot, 2, &isnull));
Assert(!isnull);
+ relkind = DatumGetChar(slot_getattr(slot, 3, &isnull));
+ Assert(!isnull);
+
+ /*
+ * Newer PG versions allow things that aren't plain tables to appear in
+ * publications. We don't handle that in this version, but try to provide
+ * a useful error message.
+ */
+ if (relkind != RELKIND_RELATION)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot sync from non-table relation \"%s.%s\"",
+ nspname, relname)));
ExecDropSingleTupleTableSlot(slot);
walrcv_clear_result(res);
* Re: Better error message for unsupported replication cases
@ 2022-02-15 03:26 Amit Kapila <amit.kapila16@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Amit Kapila @ 2022-02-15 03:26 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Feb 15, 2022 at 3:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> In [1] there's a complaint that if you try to logically replicate
> a partitioned table from v13-or-later to v12-or-earlier, you get
> "table XXX not found on publisher", which is pretty confusing
> because the publisher certainly does have such a table. That
> happens because fetch_remote_table_info is too aggressive about
> filtering by relkind and doesn't see the relation at all.
> c314c147c improved that, but it wasn't back-patched. I propose
> putting the attached into v10-v12. Maybe the error message
> could be bikeshedded ... is "non-table relation" terminology
> that we use in user-facing messages?
>
The other option could be "logical replication source relation
\"%s.%s\" is not a table". We use a similar message in
CheckSubscriptionRelkind.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Better error message for unsupported replication cases
@ 2022-02-15 03:58 Tom Lane <tgl@sss.pgh.pa.us>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2022-02-15 03:58 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Amit Kapila <amit.kapila16@gmail.com> writes:
> On Tue, Feb 15, 2022 at 3:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... Maybe the error message
>> could be bikeshedded ... is "non-table relation" terminology
>> that we use in user-facing messages?
> The other option could be "logical replication source relation
> \"%s.%s\" is not a table". We use a similar message in
> CheckSubscriptionRelkind.
Works for me, I'll do it like that if there are no objections.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-15 03:58 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-14 22:12 Better error message for unsupported replication cases Tom Lane <tgl@sss.pgh.pa.us>
2022-02-15 03:26 ` Amit Kapila <amit.kapila16@gmail.com>
2022-02-15 03:58 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox