On Thu, Mar 26, 2020 at 11:23 PM Amit Langote <amitlangot...@gmail.com> wrote: > 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.
Turns out the code in apply_handle_tuple_routing() for the UPDATE message was somewhat bogus, which fixed in the updated version. I ended up with anothing refactoring patch, which attached as 0001. It appears to me that the tests now seem enough to cover apply_handle_tuple_routing(), although more could still be added. > > 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. Removed that memset. I have added a comment about one- vs. zero-based indexes contained in the maps coming from two different modules, viz. tuple routing and logical replication, resp. -- Thank you, Amit Langote EnterpriseDB: http://www.enterprisedb.com
v14-0003-Publish-partitioned-table-inserts-as-its-own.patch
Description: Binary data
v1-0001-worker.c-refactor-code-to-look-up-local-tuple.patch
Description: Binary data
v14-0002-Add-subscription-support-to-replicate-into-parti.patch
Description: Binary data
