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