From Mon, Aug 30, 2021 3:07 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > I've attached rebased patches. 0004 patch is not the scope of this patch. It's > borrowed from another thread[1] to fix the assertion failure for newly added > tests. Please review them.
Hi, I reviewed the v12-0001 patch, here are some comments: 1) --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -1441,7 +1441,6 @@ getinternalerrposition(void) return edata->internalpos; } - It seems a miss change in elog.c 2) + TupleDescInitEntry(tupdesc, (AttrNumber) 10, "stats_reset", + TIMESTAMPTZOID, -1, 0); The document doesn't mention the column "stats_reset". 3) +typedef struct PgStat_StatSubErrEntry +{ + Oid subrelid; /* InvalidOid if the apply worker, otherwise + * the table sync worker. hash table key. */ From the comments of subrelid, I think one subscription only have one apply worker error entry, right ? If so, I was thinking can we move the the apply error entry to PgStat_StatSubEntry. In that approach, we don't need to build a inner hash table when there are no table sync error entry. 4) Is it possible to add some testcases to test the subscription error entry delete ? Best regards, Hou zj