Hi Sawada-san,
Sorry for my late response.
On 2017/12/05 0:11, Masahiko Sawada wrote:
There is one more case that user-defined data type is not supported in Logical
Replication.
That is when remote data type's name does not exist in SUBSCRIBE.
In relation.c:logicalrep_typmap_gettypname
We search OID in syscache by remote's data type name and mapping it, if it does
not exist in syscache
We will be faced with the bellow error.
if (!OidIsValid(entry->typoid))
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("data type \"%s.%s\" required for logical
replication does not exist",
entry->nspname,
entry->typname)));
I think, it is not necessary to check typoid here in order to avoid above case,
is that right?
I think it's not right. We should end up with an error in the case
where the same type name doesn't exist on subscriber. With your
proposed patch, logicalrep_typmap_gettypname() can return an invalid
string (entry->typname) in that case, which can be a cause of SEGV.
Thanks, I think you are right.
# I thought that entry->typname was received from walsender, and it is
already be qualified in logicalrep_write_typ.
# But we also need check it in subscriber, because it could be lost info
in transmission.
Also we do not need to fix for the case above too, because user can
change type name by ALTER TYPE statement.
I attached the patch, which based on your patch with a bit of modified
messages.
---
Thanks and best regards,
Dang Minh Huong
diff --git a/src/backend/replication/logical/relation.c
b/src/backend/replication/logical/relation.c
index 46e515e4b6..0ae676a8d6 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -441,8 +441,8 @@ logicalrep_typmap_update(LogicalRepTyp *remotetyp)
/*
* Fetch type info from the cache.
*/
-Oid
-logicalrep_typmap_getid(Oid remoteid)
+char *
+logicalrep_typmap_gettypname(Oid remoteid)
{
LogicalRepTyp *entry;
bool found;
@@ -453,9 +453,9 @@ logicalrep_typmap_getid(Oid remoteid)
{
if (!get_typisdefined(remoteid))
ereport(ERROR,
- (errmsg("built-in type %u not found",
remoteid),
+ (errmsg("built-in type oid %u not
found", remoteid),
errhint("This can be caused by having
a publisher with a higher PostgreSQL major version than the subscriber.")));
- return remoteid;
+ return format_type_be(remoteid);
}
if (LogicalRepTypMap == NULL)
@@ -466,12 +466,12 @@ logicalrep_typmap_getid(Oid remoteid)
HASH_FIND, &found);
if (!found)
- elog(ERROR, "no type map entry for remote type %u",
+ elog(ERROR, "no type map entry for remote type oid %u",
remoteid);
- /* Found and mapped, return the oid. */
+ /* Found and mapped, return the type name. */
if (OidIsValid(entry->typoid))
- return entry->typoid;
+ return entry->typname;
/* Otherwise, try to map to local type. */
nspoid = LookupExplicitNamespace(entry->nspname, true);
@@ -488,5 +488,6 @@ logicalrep_typmap_getid(Oid remoteid)
errmsg("data type \"%s.%s\" required for
logical replication does not exist",
entry->nspname,
entry->typname)));
- return entry->typoid;
+ /* Return type name */
+ return entry->typname;
}
diff --git a/src/backend/replication/logical/worker.c
b/src/backend/replication/logical/worker.c
index eedc3a8816..6cf037684b 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -100,8 +100,9 @@ static dlist_head lsn_mapping =
DLIST_STATIC_INIT(lsn_mapping);
typedef struct SlotErrCallbackArg
{
- LogicalRepRelation *rel;
- int attnum;
+ LogicalRepRelMapEntry *rel;
+ int remote_attnum;
+ int local_attnum;
} SlotErrCallbackArg;
static MemoryContext ApplyMessageContext = NULL;
@@ -282,17 +283,20 @@ slot_store_error_callback(void *arg)
SlotErrCallbackArg *errarg = (SlotErrCallbackArg *) arg;
Oid remotetypoid,
localtypoid;
+ char *remotetypname;
- if (errarg->attnum < 0)
+ if (errarg->remote_attnum < 0)
return;
- remotetypoid = errarg->rel->atttyps[errarg->attnum];
- localtypoid = logicalrep_typmap_getid(remotetypoid);
+ remotetypoid = errarg->rel->remoterel.atttyps[errarg->remote_attnum];
+ remotetypname = logicalrep_typmap_gettypname(remotetypoid);
+ localtypoid = get_atttype(errarg->rel->localreloid,
errarg->local_attnum + 1);
+
errcontext("processing remote data for replication target relation
\"%s.%s\" column \"%s\", "
- "remote type %s, local type %s",
- errarg->rel->nspname, errarg->rel->relname,
- errarg->rel->attnames[errarg->attnum],
- format_type_be(remotetypoid),
+ "remote type \"%s\", local type \"%s\"",
+ errarg->rel->remoterel.nspname,
errarg->rel->remoterel.relname,
+
errarg->rel->remoterel.attnames[errarg->remote_attnum],
+ remotetypname,
format_type_be(localtypoid));
}
@@ -313,8 +317,9 @@ slot_store_cstrings(TupleTableSlot *slot,
LogicalRepRelMapEntry *rel,
ExecClearTuple(slot);
/* Push callback + info on the error context stack */
- errarg.rel = &rel->remoterel;
- errarg.attnum = -1;
+ errarg.rel = rel;
+ errarg.local_attnum = -1;
+ errarg.remote_attnum = -1;
errcallback.callback = slot_store_error_callback;
errcallback.arg = (void *) &errarg;
errcallback.previous = error_context_stack;
@@ -332,7 +337,8 @@ slot_store_cstrings(TupleTableSlot *slot,
LogicalRepRelMapEntry *rel,
Oid typinput;
Oid typioparam;
- errarg.attnum = remoteattnum;
+ errarg.local_attnum = i;
+ errarg.remote_attnum = remoteattnum;
getTypeInputInfo(att->atttypid, &typinput, &typioparam);
slot->tts_values[i] = OidInputFunctionCall(typinput,
@@ -378,8 +384,9 @@ slot_modify_cstrings(TupleTableSlot *slot,
LogicalRepRelMapEntry *rel,
ExecClearTuple(slot);
/* Push callback + info on the error context stack */
- errarg.rel = &rel->remoterel;
- errarg.attnum = -1;
+ errarg.rel = rel;
+ errarg.local_attnum = -1;
+ errarg.remote_attnum = -1;
errcallback.callback = slot_store_error_callback;
errcallback.arg = (void *) &errarg;
errcallback.previous = error_context_stack;
@@ -402,7 +409,8 @@ slot_modify_cstrings(TupleTableSlot *slot,
LogicalRepRelMapEntry *rel,
Oid typinput;
Oid typioparam;
- errarg.attnum = remoteattnum;
+ errarg.local_attnum = i;
+ errarg.remote_attnum = remoteattnum;
getTypeInputInfo(att->atttypid, &typinput, &typioparam);
slot->tts_values[i] = OidInputFunctionCall(typinput,
diff --git a/src/include/replication/logicalrelation.h
b/src/include/replication/logicalrelation.h
index 8352705650..563bb4f37d 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -37,6 +37,6 @@ extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
LOCKMODE lockmode);
extern void logicalrep_typmap_update(LogicalRepTyp *remotetyp);
-extern Oid logicalrep_typmap_getid(Oid remoteid);
+extern char *logicalrep_typmap_gettypname(Oid remoteid);
#endif /* LOGICALRELATION_H */
