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

Reply via email to