On Thu, Mar 19, 2020 at 11:18 PM Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 2020-03-18 08:33, Amit Langote wrote: > > By the way, I have rebased the patches, although maybe you've got your > > own copies; attached. > > Looking through 0002 and 0003 now. > > The structure looks generally good.
Thanks for the review. > In 0002, the naming of apply_handle_insert() vs. > apply_handle_do_insert() etc. seems a bit prone to confusion. How about > something like apply_handle_insert_internal()? Also, should we put each > of those internal functions next to their internal function instead of > in a separate group like you have it? Sure. > In apply_handle_do_insert(), the argument localslot should probably be > remoteslot. You're right, fixed. > In apply_handle_do_delete(), the ExecOpenIndices() call was moved to a > different location relative to the rest of the code. That was probably > not intended. Fixed. > In 0003, you have /* TODO, use inverse lookup hashtable? */. Is this > something you plan to address in this cycle, or is that more for future > generations? Sorry, this is simply a copy-paste from logicalrep_relmap_invalidate_cb(). > 0003 could use some more tests. The one test that you adjusted just > ensures the data goes somewhere instead of being rejected, but there are > no tests that check whether it ends up in the right partition, whether > cross-partition updates work etc. Okay, added some tests. Attached updated patches. -- Thank you, Amit
v13-0003-Publish-partitioned-table-inserts-as-its-own.patch
Description: Binary data
v13-0001-Some-refactoring-of-logical-worker.c.patch
Description: Binary data
v13-0002-Add-subscription-support-to-replicate-into-parti.patch
Description: Binary data
