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

On Sat, Feb 2, 2013 at 4:38 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-01-28 16:55:52 -0500, Steve Singer wrote:
>> If your using non-surragate /natural primary keys this tends to come up
>> occasionally due to data-entry errors or renames.  I'm  looking at this from
>> the point of view of what do I need to use this as a source for a production
>> replication system with fewer sharp-edges compared to trigger source slony.
>> My standard is a bit higher than 'first' version because I intent to use it
>> in the version 3.0 of slony not 1.0.  If others feel I'm asking for too much
>> they should speak up, maybe I am. Also the way things will fail if someone
>> were to try and update a primary key value is pretty nasty (it will leave
>> them with inconsistent databases).    We could  install UPDATE triggers to
>> try and detect this type of thing but I'd rather see us just log the old
>> values so we can use them during replay.
>
> I pushed support for this. I am not yet 100% happy with this due to two
> issues:
>
> * it increases the xlog size logged by heap_update by 2 bytes even with
>   wal_level < logical as it uses a variant of xl_heap_header that
>   includes its lenght. Conditionally using xl_heap_header would make the
>   code even harder to read. Is that acceptable?

I think it's important to avoid adding to DML WAL volume when
wal_level < logical.  I am not positive that 2 bytes is noticeable,
but I'm not positive that it isn't either: heap insert/update must be
our most commonly-used WAL records.  On the other hand, we also need
to keep in mind that branches in hot code paths aren't free either.  I
would be concerned more about the increased run-time cost of
constructing the correct WAL record than with the related code
complexity.  None of that code is simple anyway.

> * multi_insert should be converted to use xl_heap_header_len as well,
>   instead of using xl_multi_insert_tuple, that would also reduce the
>   amount of multi-insert specific code in decode.c
> * both for update and delete we should denote more explicitly that
>   ->oldtuple points to an index tuple, not to an full tuple

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company