Re: fast defaults in heap_getattr vs heap_deform_tuple

Hi,

On 2019-02-06 10:25:56 +0000, Andrew Gierth wrote:
> >>>>> "Andres" == Andres Freund <andres@anarazel.de> writes:
>
>  >> Cool. Here's the patch that I, after some commit message polishing,
>  >> plan to commit to 11 and master to fix the issue at hand.
>
>  Andres> And pushed.
>
> Sorry I didn't spot this earlier, but... in the backpatch to v11, is the
> expansion of the trigger tuple really unnecessary now? Since
> heap_getattr is a macro, C-language triggers that aren't recompiled
> won't see the default? So perhaps the expansion should be left in?

There was some IRC conversation about this:

RhodiumToad | andres: ping?
RhodiumToad | n/m, sent mail
     andres | RhodiumToad: IDK, there's a lot of other heap_getattr calls, and most of them don't go
            | through GetTupleForTrigger(). So extensions need to be recompiled either way.
     andres | (I'll write that in a bit on list, in meeting now)
RhodiumToad | right, but those other calls were already broken.
RhodiumToad | whereas in a trigger, they'd have worked previously, but then break with your fix
       Myon | the list of packages in Debian where heap_getattr appears is postgresql-11, pglogical,
            | pgextwlist, citus, postgresql-plsh, wal2json, pg-cron, postgresql-rum
RhodiumToad | it's not an encouraged interface

I'm mildly disinclined to re-add the heap_expand_tuple, because it's not
the only case, and the extensions named above don't seem like they'd
specifically affected by the trigger path, but I'm not sure.

Greetings,

Andres Freund