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

On 2013-01-26 16:20:33 -0500, Steve Singer wrote:
> 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.

> + extern void relationFindPrimaryKey(Relation pkrel, Oid *indexOid,
> + int16 *nratts, int16 *attnums, Oid
> *atttypids,
> + Oid *opclasses);
> +
>
> I don't see this defined anywhere could it be left over from a previous
> version of the patch?

Yes, its dead and now gone.

> 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?

Nope, there isn't any right now. I have considered as something not all
that interesting for real-world usecases based on my experience, but
adding support shouldn't be that hard anymore, so I can just bite the
bullet...

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

Yea, I had forgotten to add that hunk when committing. Fixed.

Thanks,

Andres

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services