Re: WIP: Pg_upgrade - page layout converter (PLC) hook

Heikki Linnakangas napsal(a):
> Zdenek Kotala wrote:
>> Heikki Linnakangas napsal(a):
>>> I would suggest starting with putting some serious effort into
>>> pg_migrator. This page layout conversion is actually pretty trivial
>>> compared to all that, and even more so if you can get the page layout
>>> conversion working in pg_migrator first.
>>
>> By my opinion pg_migrator is good workaround but it is not suitable
>> for real production. For example TOAST relid protection is dirty hack
>> and you have problem with tablespaces and so on...
>
> Sure, it's not perfect, that's exactly why I suggested working on it! If
> you have something else that works better, that's fine, but you will
> need to release it under the BSD license if you want help with it.

Sure, my upgrade script is in opensolaris and it is under CDDL license. By my
meaning it is only workaround until PostgreSQL 8.x will not have upgrade
integrated inside. After that this script will be removed.

Of course all work will be under BSD.

>>> Which versions do you plan to support, initially?
>>
>> Currently I'm able to upgrade from 8.1, 8.2 to 8.3, 8.4. It means
>> conversion between layout version 3 and 4. I'm verifying PLC now and
>> when this part will be ready it is very easy to implement others as well.
>
> Hmm. I don't see any of that code in the directory you posted. ?

PLC is not there yet, because it is not reviewed, tested and verified. And I
tested it only on SPARC where varlen works without any modification, but on x86
it needs extra work yet.

>>>> b) log_newpage is used for new page logging, but I use it for
>>>> storing converted page. It seems to me that it safe and
>>>> heap_xlog_newpage correctly works for new and converted page. I have
>>>> only doubt about assert macro mdextend/mdwrite which checks extend
>>>> vs.write.
>>>
>>> That should be fine. In WAL replay, we don't distinguish between
>>> write and extend.
>>
>> But in this case the asserts macros in mdexted/mdwrite are odd and the
>> should be removed.
>
> Those asserts are enforced during normal operation. It's just in WAL
> replay that we extend the relation automatically when full pages are
> restored. See XLogReadBuffer().

OK

>>>> + /* Hook for page layout convertor plugin */
>>>> + typedef void (*plc_hook_type)(char *buffer);
>>>> + extern PGDLLIMPORT plc_hook_type plc_hook;
>>>
>>> That's not flexible enough. We've changed the internal
>>> representations of data types in the past, and will likely do that in
>>> the future. The hook therefore needs to have at least the tuple
>>> descriptor, to know how to convert the tuples. I would suggest
>>> passing Relation, we have that available at the call site, and that
>>> should contain all the necessary information.
>>
>> Good point, I thought about it. I currently don't use it but in the
>> future it could be useful. I will extend it.
>
> Surely you need it already to do the packed varlen conversion?
>

It works on SPARC without any varlen modification :-). I originally planed to
implement varlen modification in another way but it seems to be better to do it
in this place as well.

Thanks for your comment