Re: fast defaults in heap_getattr vs heap_deform_tuple

Hi,

On 2019-02-06 03:39:19 -0800, Andres Freund wrote:
> On 2019-02-06 10:25:56 +0000, Andrew Gierth wrote:
> > >>>>> "Andres" == Andres Freund <andres(at)anarazel(dot)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.

I put the expansion back for 11. Seems harmless enough. I'm planning to
send a reply to the minor release info to -packagers informing people
that extensions compiled after the minor release might not be compatible
with older servers (because they depend on getmissingattr() which
previously wasn't an exposed function)

Greetings,

Andres Freund