Re: PATCH: 9.5 replication origins fix for logical decoding

On October 15, 2015 1:02:04 PM GMT+02:00, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>On 15 October 2015 at 17:43, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>>> I guess since it'll be InvalidRepOriginId otherwise, that makes
>sense.
>>
>> That's not the point. XACT_XINFO_HAS_ORIGIN is about
>> origin_lsn/timestamp, it doesn't have anything to do with the record
>> origin (which is included in many more types of record than just
>> commits).
>
>Ok, I think I see. That's also why it wasn't incorporated into
>xl_xact_parsed_commit.
>
>I'll check which records can contain it and assign it in the
>appropriate decoding calls. I'll follow up in a while with an updated
>patch.

As far as I can see all the other places have it assigned.

>>> > Your test prints the origins from the transaction instead the
>changes -
>>> > why?
>>>
>>> I don't understand this part.
>>
>> Your test prints origin in commits - but changes can have individual
>> origins.
>
>I was completely unaware of that, so thankyou. I'll change the tests
>to exercise that. Any preferences on the output format?
>
>Maybe:
>
>table public.origin_tbl: INSERT: id[integer]:6 data[text]:'from second
>origin' -- origin:'some_origin' origin_lsn:'0/1234'
>
>?
>
>it's cluttered, but really I'm not sure there's a pretty way to pack
>that in, and it's only test output.

I'm inclined not to commit this part - seems to add too much complications for the amount of coverage. But please use it for testing.

Andres

---
Please excuse brevity and formatting - I am writing this on my mobile phone.