Re: logical changeset generation v4 - Heikki's thoughts about the patch state

On 13-01-24 11:15 AM, Steve Singer wrote:
> On 13-01-24 06:40 AM, Andres Freund wrote:
>>
>> Fair enough. I am also working on a user of this infrastructure but that
>> doesn't help you very much. Steve Singer seemed to make some stabs at
>> writing an output plugin as well. Steve, how far did you get there?
>
> I was able to get something that generated output for INSERT
> statements in a format similar to what a modified slony apply trigger
> would want. This was with the list of tables to replicate hard-coded
> in the plugin. This was with the patchset from the last commitfest.I
> had gotten a bit hung up on the UPDATE and DELETE support because
> slony allows you to use an arbitrary user specified unique index as
> your key. It looks like better support for tables with a unique
> non-primary key is in the most recent patch set. I am hoping to have
> time this weekend to update my plugin to use parameters passed in on
> the init and other updates in the most recent version. If I make some
> progress I will post a link to my progress at the end of the weekend.
> My big issue is that I have limited time to spend on this.
>

This isn't a complete review just a few questions I've hit so far that I
thought I'd ask to see if I'm not seeing something related to updates.

*** a/src/include/catalog/index.h
--- b/src/include/catalog/index.h
*************** extern bool ReindexIsProcessingHeap(Oid
*** 114,117 ****
--- 114,121 ----
extern bool ReindexIsProcessingIndex(Oid indexOid);
extern Oid IndexGetRelation(Oid indexId, bool missing_ok);

+ extern void relationFindPrimaryKey(Relation pkrel, Oid *indexOid,
+ int16 *nratts, int16 *attnums, Oid
*atttypids,
+ Oid *opclasses);
+
#endif /* INDEX_H */

I don't see this defined anywhere could it be left over from a previous
version of the patch?

In decode.c
DecodeUpdate:
+
+ /*
+ * FIXME: need to get/save the old tuple as well if we want primary key
+ * changes to work.
+ */
+ change->newtuple = ReorderBufferGetTupleBuf(reorder);

I also don't see any code in heap_update to find + save the old primary
key values like you added to heap_delete. You didn't list "Add ability
to change the primary key on an UPDATE" in the TODO so I'm wondering if
I'm missing something. Is there another way I can bet the primary key
values for the old_tuple?

Also,

I think the name of the test contrib module was changed but you didn't
update the make file. This fixes it

diff --git a/contrib/Makefile b/contrib/Makefile
index 1cc30fe..36e6bfe 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -50,7 +50,7 @@ SUBDIRS = \
tcn \
test_parser \
test_decoding \
- test_logical_replication \
+ test_logical_decoding \
tsearch2 \
unaccent \
vacuumlo \