From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: fast defaults in heap_getattr vs heap_deform_tuple |
Date: | 2019-02-01 22:49:05 |
Message-ID: | 20190201224905.3qkbm5gj6ielsszc@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2019-02-01 08:24:04 -0800, Andres Freund wrote:
> While working on the patch to slotify trigger.c I got somewhat confused
> by the need to expand tuples in trigger.c:
>
> static HeapTuple
> GetTupleForTrigger(EState *estate,
> EPQState *epqstate,
> ResultRelInfo *relinfo,
> ItemPointer tid,
> LockTupleMode lockmode,
> TupleTableSlot **newSlot)
> {
> ...
> if (HeapTupleHeaderGetNatts(tuple.t_data) < relation->rd_att->natts)
> result = heap_expand_tuple(&tuple, relation->rd_att);
> else
> result = heap_copytuple(&tuple);
> ReleaseBuffer(buffer);
>
> There's no explanation why GetTupleForTrigger() needs expanding tuples,
> but most other parts of postgres don't. Normally deforming ought to take
> care of expanding missing attrs.
>
> As far as I can tell, the reason that it's needed is that heap_gettar()
> wasn't updated along the rest of the functions. heap_deform_tuple(),
> heap_attisnull() etc look for missing attrs, but heap_getattr() doesn't.
>
> That, to me, makes no sense. The reason that we see a difference in
> regression test output is that composite_to_json() uses heap_getattr(),
> if it used heap_deform_tuple (which'd be considerably faster), we'd get
> the default value.
>
> Am I missing something?
Indeed, a hacky patch fixing heap_getattr makes the heap_expand_tuple()
in trigger.c unnecessary. Attached. I think we need to do something
along those lines.
Andrew, I think it'd be good to do a ground up review of the fast
defaults patch. There's been a fair number of issues in it, and this is
a another pretty fundamental issue.
Greetings,
Andres Freund
Attachment | Content-Type | Size |
---|---|---|
poc-fixme-heap_getattr.diff | text/x-diff | 2.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Chengchao Yu | 2019-02-01 23:12:39 | RE: [PATCH] Fix Proposal - Deadlock Issue in Single User Mode When IO Failure Occurs |
Previous Message | Bossart, Nathan | 2019-02-01 22:43:37 | Re: New vacuum option to do only freezing |