Avoid needless copy in nodeMaterial

Started by Neil Conwayalmost 19 years ago4 messagespatches
Jump to latest
#1Neil Conway
neilc@samurai.com

Attached is a patch that avoids a needless copy of the result tuple in
nodeMaterial, in the case that we don't have a previously-materialized
tuple to return. We can just return the TTS produced by executing our
child node, rather than returning a copy of it.

I didn't bother pulling the MinimalTuple out of "outerslot" and stuffing
it back into the nodeMaterial's result slot, as AFAICS that is not
necessary. Although I suppose you could make a cleanliness argument that
that would be worth doing instead.

(This is 8.4 material...)

-Neil

Attachments:

material_copy_tweak-1.patchtext/x-patch; charset=UTF-8; name=material_copy_tweak-1.patchDownload+20-18
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#1)
Re: Avoid needless copy in nodeMaterial

Neil Conway <neilc@samurai.com> writes:

Attached is a patch that avoids a needless copy of the result tuple in
nodeMaterial, in the case that we don't have a previously-materialized
tuple to return.

Seems like this needs more comments about what's happening, rather
than less ...

Also, it looks to me like the plan node's own resultslot might never be
assigned to at all, when the subplan returns zero rows. Does this
corner case still work correctly?

regards, tom lane

#3Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#2)
Re: Avoid needless copy in nodeMaterial

On Tue, 2007-10-16 at 00:34 -0400, Tom Lane wrote:

Seems like this needs more comments about what's happening, rather
than less ...

Fair point.

Also, it looks to me like the plan node's own resultslot might never be
assigned to at all, when the subplan returns zero rows. Does this
corner case still work correctly?

ISTM the node's own result slot wouldn't be assigned to in this case
regardless: (nodeMaterial.c, circa 116)

outerslot = ExecProcNode(outerNode);
if (TupIsNull(outerslot))
{
node->eof_underlying = true;
return NULL;
}

There's no requirement that we must assign to the result slot, AFAICS.

-Neil

#4Bruce Momjian
bruce@momjian.us
In reply to: Neil Conway (#1)
Re: Avoid needless copy in nodeMaterial

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Neil Conway wrote:

Attached is a patch that avoids a needless copy of the result tuple in
nodeMaterial, in the case that we don't have a previously-materialized
tuple to return. We can just return the TTS produced by executing our
child node, rather than returning a copy of it.

I didn't bother pulling the MinimalTuple out of "outerslot" and stuffing
it back into the nodeMaterial's result slot, as AFAICS that is not
necessary. Although I suppose you could make a cleanliness argument that
that would be worth doing instead.

(This is 8.4 material...)

-Neil

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +