Thread: Avoid needless copy in nodeMaterial

Avoid needless copy in nodeMaterial

From
Neil Conway
Date:
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

Re: Avoid needless copy in nodeMaterial

From
Tom Lane
Date:
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

Re: Avoid needless copy in nodeMaterial

From
Neil Conway
Date:
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



Re: Avoid needless copy in nodeMaterial

From
Bruce Momjian
Date:
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. +