Re: [PATCHES] avg(int2) and avg(int8) micro-opt

2005-04-06 Thread Neil Conway
Neil Conway wrote:
Attached is a patch that applies the same optimization to int2_sum(), 
int4_sum(), float4_accum(), and float8_accum(). It wasn't possible to 
optimize do_numeric_accum() or int8_sum() since they both use numerics. 
Applied.
-Neil
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
 joining column's datatypes do not match


[PATCHES] avg(int2) and avg(int8) micro-opt

2005-04-04 Thread Neil Conway
This patch changes int2_avg_accum() and int4_avg_accum() use the nodeAgg 
performance hack Tom introduced recently. This means we can avoid 
copying the transition array for each input tuple if these functions are 
invoked as aggregate transition functions.

To test the performance improvement, I created a 1 million row table 
with a single int4 column. Without the patch, SELECT avg(col) FROM table 
took about 4.2 seconds (after the data was cached); with the patch, it 
took about 3.2 seconds. Naturally, the performance improvement for a 
less trivial query (or a table with wider rows) would be relatively smaller.

It is possible that the transition array might be TOAST'ed (not that I'd 
expect that to occur in practice, of course). The aggregates should 
continue to work in this case: PG_DETOAST_DATUM() is equivalent to 
PG_DETOAST_DATUM_COPY() if the datum is toast'ed, so in effect we just 
won't implement the nodeAgg performance hack if the transition array is 
toasted. If I've mucked this up, let me know.

Barring any objections, I'll commit this tomorrow.
-Neil
Index: src/backend/utils/adt/numeric.c
===
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.81
diff -c -r1.81 numeric.c
*** src/backend/utils/adt/numeric.c	1 Jan 2005 05:43:07 -	1.81
--- src/backend/utils/adt/numeric.c	4 Apr 2005 11:00:41 -
***
*** 2462,2478 
  Datum
  int2_avg_accum(PG_FUNCTION_ARGS)
  {
! 	ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
  	int16		newval = PG_GETARG_INT16(1);
  	Int8TransTypeData *transdata;
  
  	/*
! 	 * We copied the input array, so it's okay to scribble on it directly.
  	 */
  	if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData))
  		elog(ERROR, expected 2-element int8 array);
- 	transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
  
  	transdata-count++;
  	transdata-sum += newval;
  
--- 2462,2485 
  Datum
  int2_avg_accum(PG_FUNCTION_ARGS)
  {
! 	ArrayType  *transarray;
  	int16		newval = PG_GETARG_INT16(1);
  	Int8TransTypeData *transdata;
  
  	/*
! 	 * If we're invoked by nodeAgg, we can cheat and modify our first
! 	 * parameter in-place to reduce palloc overhead. Otherwise we need
! 	 * to make a copy of it before scribbling on it.
  	 */
+ 	if (fcinfo-context  IsA(fcinfo-context, AggState))
+ 		transarray = PG_GETARG_ARRAYTYPE_P(0);
+ 	else
+ 		transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
+ 
  	if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData))
  		elog(ERROR, expected 2-element int8 array);
  
+ 	transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
  	transdata-count++;
  	transdata-sum += newval;
  
***
*** 2482,2498 
  Datum
  int4_avg_accum(PG_FUNCTION_ARGS)
  {
! 	ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
  	int32		newval = PG_GETARG_INT32(1);
  	Int8TransTypeData *transdata;
  
  	/*
! 	 * We copied the input array, so it's okay to scribble on it directly.
  	 */
  	if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData))
  		elog(ERROR, expected 2-element int8 array);
- 	transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
  
  	transdata-count++;
  	transdata-sum += newval;
  
--- 2489,2512 
  Datum
  int4_avg_accum(PG_FUNCTION_ARGS)
  {
! 	ArrayType  *transarray;
  	int32		newval = PG_GETARG_INT32(1);
  	Int8TransTypeData *transdata;
  
  	/*
! 	 * If we're invoked by nodeAgg, we can cheat and modify our first
! 	 * parameter in-place to reduce palloc overhead. Otherwise we need
! 	 * to make a copy of it before scribbling on it.
  	 */
+ 	if (fcinfo-context  IsA(fcinfo-context, AggState))
+ 		transarray = PG_GETARG_ARRAYTYPE_P(0);
+ 	else
+ 		transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
+ 
  	if (ARR_SIZE(transarray) != ARR_OVERHEAD(1) + sizeof(Int8TransTypeData))
  		elog(ERROR, expected 2-element int8 array);
  
+ 	transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
  	transdata-count++;
  	transdata-sum += newval;
  

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] avg(int2) and avg(int8) micro-opt

2005-04-04 Thread Tom Lane
Neil Conway [EMAIL PROTECTED] writes:
 This patch changes int2_avg_accum() and int4_avg_accum() use the nodeAgg 
 performance hack Tom introduced recently.

Why only those two?  Might as well make all the accum functions look alike.

 It is possible that the transition array might be TOAST'ed (not that I'd 
 expect that to occur in practice, of course).

AFAICS that is impossible, since the transition value is never stored to
disk.  But your analysis is good anyway.

regards, tom lane

---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings


Re: [PATCHES] avg(int2) and avg(int8) micro-opt

2005-04-04 Thread Neil Conway
Tom Lane wrote:
Why only those two?  Might as well make all the accum functions look alike.
Yeah, there might be some others we could improve. float4_accum() and 
float8_accum() look like they could be improved pretty easily, and 
do_numeric_accum() should also be fixable with some hackery. I suppose 
it's also worth optimizing int2_sum(), int4_sum() and int8_sum(). I'll 
send a patch for this later today or tomorrow. Are there any other 
transition functions where we can apply this technique?

BTW, int2_avg_accum() and int4_avg_accum() patch applied to HEAD.
-Neil
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
 joining column's datatypes do not match


Re: [PATCHES] avg(int2) and avg(int8) micro-opt

2005-04-04 Thread Tom Lane
Neil Conway [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 Why only those two?  Might as well make all the accum functions look alike.

 Yeah, there might be some others we could improve. float4_accum() and 
 float8_accum() look like they could be improved pretty easily, and 
 do_numeric_accum() should also be fixable with some hackery. I suppose 
 it's also worth optimizing int2_sum(), int4_sum() and int8_sum(). I'll 
 send a patch for this later today or tomorrow. Are there any other 
 transition functions where we can apply this technique?

Actually, do_numeric_accum can't be fixed easily because numeric is a
varlena type.  The basic requirement for this hack is that the size of
the transition value be constant ...

regards, tom lane

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq