On Thu, Dec 19, 2013 at 9:37 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Dec 19, 2013 at 9:19 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> On 2013-12-19 07:40:40 -0500, Robert Haas wrote:
>>> On Thu, Dec 19, 2013 at 5:44 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>>> > On 2013-12-18 21:42:25 -0500, Robert Haas wrote:
>>>
>>> > I dislike transporting the
>>> > infomask in the wal record and then changing it away from that again afterwards.
>>>
>>> I don't really see a problem with it. Relying on the macros to tweak
>>> the bits seems more future-proof than inventing some other way to do
>>> it (that might even get copied into other parts of the code where it's
>>> even less safe).
>>
>> Then there should be a macro to twiddle the infomask, without touching
>> the tuple.
>
> Sure, we can invent that. I personally don't like it as well.
After some off-list discussion via IM I propose the following
compromise: it reverts my changes to do some of the infomask bit
twaddling in the execute function, but doesn't invent new macros
either. My main concern about inventing new macros is that I don't
want to encourage people to write code that knows specifically which
parts of the heap tuple header are in which fields. I think it may
have been a mistake to divide responsibility between the prepare and
execute functions the way we did in this case, because it doesn't
appear to be a clean separation of concerns. But it's not this
patch's job to kibitz that decision, so this version just fits in with
the way things are already being done there.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company