Re: [PATCHES] Avoid needless copy in nodeMaterial

[Bruce Momjian]

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  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://postgres.enterprisedb.com

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

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

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

Re: [PATCHES] Avoid needless copy in nodeMaterial

[Neil Conway]

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



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Avoid needless copy in nodeMaterial

[Tom Lane]

Neil Conway <[EMAIL PROTECTED]> 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

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly

[PATCHES] Avoid needless copy in nodeMaterial

[Neil Conway]

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

Index: source/src/backend/executor/nodeMaterial.c
===
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/executor/nodeMaterial.c,v
retrieving revision 1.59
diff -p -c -r1.59 nodeMaterial.c
*** source/src/backend/executor/nodeMaterial.c	21 May 2007 17:57:33 -	1.59
--- source/src/backend/executor/nodeMaterial.c	16 Oct 2007 04:00:46 -
*** ExecMaterial(MaterialState *node)
*** 98,103 
--- 98,105 
  			eof_tuplestore = true;
  	}
  
+ 	ExecClearTuple(slot);
+ 
  	/*
  	 * If necessary, try to fetch another row from the subplan.
  	 *
*** ExecMaterial(MaterialState *node)
*** 124,147 
  		}
  
  		/*
! 		 * Append returned tuple to tuplestore.  NOTE: because the tuplestore
! 		 * is certainly in EOF state, its read position will move forward over
! 		 * the added tuple.  This is what we want.
  		 */
  		if (tuplestorestate)
  			tuplestore_puttupleslot(tuplestorestate, outerslot);
  
! 		/*
! 		 * And return a copy of the tuple.	(XXX couldn't we just return the
! 		 * outerslot?)
! 		 */
! 		return ExecCopySlot(slot, outerslot);
  	}
  
! 	/*
! 	 * Nothing left ...
! 	 */
! 	return ExecClearTuple(slot);
  }
  
  /* 
--- 126,143 
  		}
  
  		/*
! 		 * Append a copy of the returned tuple to tuplestore.  NOTE: because
! 		 * the tuplestore is certainly in EOF state, its read position will
! 		 * move forward over the added tuple.  This is what we want.
  		 */
  		if (tuplestorestate)
  			tuplestore_puttupleslot(tuplestorestate, outerslot);
  
! 		return outerslot;
  	}
  
! 	/* Nothing left, return empty slot */
! 	return slot;
  }
  
  /* 

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

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