On Wed, Mar 25, 2020 at 9:29 PM Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 2020-03-23 06:02, Amit Langote wrote: > > Okay, added some tests. > > > > Attached updated patches. > > I have committed the worker.c refactoring patch. > > "Add subscription support to replicate into partitioned tables" still > has lacking test coverage. Your changes in relation.c are not exercised > at all because the partitioned table branch in apply_handle_update() is > never taken. This is critical and tricky code, so I would look for > significant testing.
While trying some tests around the code you mentioned, I found what looks like a bug, which looking into now. > The code looks okay to me. I would remove this code > > + memset(entry->attrmap->attnums, -1, > + entry->attrmap->maplen * sizeof(AttrNumber)); > > because the entries are explicitly filled right after anyway, and > filling the bytes with -1 has an unclear effect. There is also > seemingly some fishiness in this code around whether attribute numbers > are zero- or one-based. Perhaps this could be documented briefly. > Maybe I'm misunderstanding something. Will check and fix as necessary. -- Thank you, Amit Langote EnterpriseDB: http://www.enterprisedb.com
