Re: Avg performance for int8/numeric - Mailing list pgsql-patches

From Mark Kirkwood
Subject Re: Avg performance for int8/numeric
Date
Msg-id 45678E2C.4040403@paradise.net.nz
Whole thread Raw
In response to Re: Avg performance for int8/numeric  (Neil Conway <neilc@samurai.com>)
Responses Re: Avg performance for int8/numeric
Re: Avg performance for int8/numeric
List pgsql-patches
Neil Conway wrote:
> On Fri, 2006-11-24 at 11:08 +1300, Mark Kirkwood wrote:
>> - Modifies do_numeric_accum to have an extra bool parameter and does not
>> calc sumX2 when it is false.
>
> I think it would be clearer to reorganize this function slightly, and
> have only a single branch on "useSumX2". On first glance it isn't
> obviously that transdatums[2] is defined (but unchanged) when useSumX2
> is false.
>

Right - new patch attached that adds a new function do_numeric_avg_accum
that only uses N and sum(X). This means I could amend the avg aggregates
for numeric, int8 to have a initvalues of {0,0}.

Cheers

Mark
? gmon.out
Index: src/backend/utils/adt/numeric.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.96
diff -c -r1.96 numeric.c
*** src/backend/utils/adt/numeric.c    4 Oct 2006 00:29:59 -0000    1.96
--- src/backend/utils/adt/numeric.c    25 Nov 2006 00:00:58 -0000
***************
*** 2097,2102 ****
--- 2097,2136 ----
      return result;
  }

+ /*
+  * Improve avg performance by not caclulating sum(X*X).
+  */
+ static ArrayType *
+ do_numeric_avg_accum(ArrayType *transarray, Numeric newval)
+ {
+     Datum       *transdatums;
+     int            ndatums;
+     Datum        N,
+                 sumX;
+     ArrayType  *result;
+
+     /* We assume the input is array of numeric */
+     deconstruct_array(transarray,
+                       NUMERICOID, -1, false, 'i',
+                       &transdatums, NULL, &ndatums);
+     if (ndatums != 2)
+         elog(ERROR, "expected 2-element numeric array");
+     N = transdatums[0];
+     sumX = transdatums[1];
+
+     N = DirectFunctionCall1(numeric_inc, N);
+     sumX = DirectFunctionCall2(numeric_add, sumX,
+                                NumericGetDatum(newval));
+
+     transdatums[0] = N;
+     transdatums[1] = sumX;
+
+     result = construct_array(transdatums, 2,
+                              NUMERICOID, -1, false, 'i');
+
+     return result;
+ }
+
  Datum
  numeric_accum(PG_FUNCTION_ARGS)
  {
***************
*** 2107,2112 ****
--- 2141,2158 ----
  }

  /*
+  * Optimized case for average of numeric.
+  */
+ Datum
+ numeric_avg_accum(PG_FUNCTION_ARGS)
+ {
+     ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
+     Numeric        newval = PG_GETARG_NUMERIC(1);
+
+     PG_RETURN_ARRAYTYPE_P(do_numeric_avg_accum(transarray, newval));
+ }
+
+ /*
   * Integer data types all use Numeric accumulators to share code and
   * avoid risk of overflow.    For int2 and int4 inputs, Numeric accumulation
   * is overkill for the N and sum(X) values, but definitely not overkill
***************
*** 2151,2156 ****
--- 2197,2218 ----
      PG_RETURN_ARRAYTYPE_P(do_numeric_accum(transarray, newval));
  }

+ /*
+  * Optimized case for average of int8.
+  */
+ Datum
+ int8_avg_accum(PG_FUNCTION_ARGS)
+ {
+     ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
+     Datum        newval8 = PG_GETARG_DATUM(1);
+     Numeric        newval;
+
+     newval = DatumGetNumeric(DirectFunctionCall1(int8_numeric, newval8));
+
+     PG_RETURN_ARRAYTYPE_P(do_numeric_avg_accum(transarray, newval));
+ }
+
+
  Datum
  numeric_avg(PG_FUNCTION_ARGS)
  {
***************
*** 2164,2174 ****
      deconstruct_array(transarray,
                        NUMERICOID, -1, false, 'i',
                        &transdatums, NULL, &ndatums);
!     if (ndatums != 3)
!         elog(ERROR, "expected 3-element numeric array");
      N = DatumGetNumeric(transdatums[0]);
      sumX = DatumGetNumeric(transdatums[1]);
-     /* ignore sumX2 */

      /* SQL92 defines AVG of no values to be NULL */
      /* N is zero iff no digits (cf. numeric_uminus) */
--- 2226,2235 ----
      deconstruct_array(transarray,
                        NUMERICOID, -1, false, 'i',
                        &transdatums, NULL, &ndatums);
!     if (ndatums != 2)
!         elog(ERROR, "expected 2-element numeric array");
      N = DatumGetNumeric(transdatums[0]);
      sumX = DatumGetNumeric(transdatums[1]);

      /* SQL92 defines AVG of no values to be NULL */
      /* N is zero iff no digits (cf. numeric_uminus) */
Index: src/include/catalog/pg_aggregate.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_aggregate.h,v
retrieving revision 1.58
diff -c -r1.58 pg_aggregate.h
*** src/include/catalog/pg_aggregate.h    4 Oct 2006 00:30:07 -0000    1.58
--- src/include/catalog/pg_aggregate.h    25 Nov 2006 00:01:01 -0000
***************
*** 80,89 ****
   */

  /* avg */
! DATA(insert ( 2100    int8_accum        numeric_avg        0    1231    "{0,0,0}" ));
  DATA(insert ( 2101    int4_avg_accum    int8_avg        0    1016    "{0,0}" ));
  DATA(insert ( 2102    int2_avg_accum    int8_avg        0    1016    "{0,0}" ));
! DATA(insert ( 2103    numeric_accum    numeric_avg        0    1231    "{0,0,0}" ));
  DATA(insert ( 2104    float4_accum    float8_avg        0    1022    "{0,0,0}" ));
  DATA(insert ( 2105    float8_accum    float8_avg        0    1022    "{0,0,0}" ));
  DATA(insert ( 2106    interval_accum    interval_avg    0    1187    "{0 second,0 second}" ));
--- 80,89 ----
   */

  /* avg */
! DATA(insert ( 2100    int8_avg_accum    numeric_avg        0    1231    "{0,0}" ));
  DATA(insert ( 2101    int4_avg_accum    int8_avg        0    1016    "{0,0}" ));
  DATA(insert ( 2102    int2_avg_accum    int8_avg        0    1016    "{0,0}" ));
! DATA(insert ( 2103    numeric_avg_accum    numeric_avg        0    1231    "{0,0}" ));
  DATA(insert ( 2104    float4_accum    float8_avg        0    1022    "{0,0,0}" ));
  DATA(insert ( 2105    float8_accum    float8_avg        0    1022    "{0,0,0}" ));
  DATA(insert ( 2106    interval_accum    interval_avg    0    1187    "{0 second,0 second}" ));
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.428
diff -c -r1.428 pg_proc.h
*** src/include/catalog/pg_proc.h    24 Nov 2006 21:18:42 -0000    1.428
--- src/include/catalog/pg_proc.h    25 Nov 2006 00:01:20 -0000
***************
*** 2697,2708 ****
--- 2697,2710 ----
  DATA(insert OID = 1832 (  float8_stddev_samp    PGNSP PGUID 12 f f t f i 1 701 "1022" _null_ _null_ _null_
float8_stddev_samp- _null_ )); 
  DESCR("STDDEV_SAMP aggregate final function");
  DATA(insert OID = 1833 (  numeric_accum    PGNSP PGUID 12 f f t f i 2 1231 "1231 1700" _null_ _null_ _null_
numeric_accum- _null_ )); 
+ DATA(insert OID = 2858 (  numeric_avg_accum    PGNSP PGUID 12 f f t f i 2 1231 "1231 1700" _null_ _null_ _null_
numeric_avg_accum- _null_ )); 
  DESCR("aggregate transition function");
  DATA(insert OID = 1834 (  int2_accum       PGNSP PGUID 12 f f t f i 2 1231 "1231 21" _null_ _null_ _null_ int2_accum
-_null_ )); 
  DESCR("aggregate transition function");
  DATA(insert OID = 1835 (  int4_accum       PGNSP PGUID 12 f f t f i 2 1231 "1231 23" _null_ _null_ _null_ int4_accum
-_null_ )); 
  DESCR("aggregate transition function");
  DATA(insert OID = 1836 (  int8_accum       PGNSP PGUID 12 f f t f i 2 1231 "1231 20" _null_ _null_ _null_ int8_accum
-_null_ )); 
+ DATA(insert OID = 2857 (  int8_avg_accum       PGNSP PGUID 12 f f t f i 2 1231 "1231 20" _null_ _null_ _null_
int8_avg_accum- _null_ )); 
  DESCR("aggregate transition function");
  DATA(insert OID = 1837 (  numeric_avg       PGNSP PGUID 12 f f t f i 1 1700 "1231" _null_ _null_ _null_
numeric_avg- _null_ )); 
  DESCR("AVG aggregate final function");
Index: src/include/utils/builtins.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/builtins.h,v
retrieving revision 1.282
diff -c -r1.282 builtins.h
*** src/include/utils/builtins.h    18 Sep 2006 22:40:40 -0000    1.282
--- src/include/utils/builtins.h    25 Nov 2006 00:01:23 -0000
***************
*** 833,841 ****
--- 833,843 ----
  extern Datum text_numeric(PG_FUNCTION_ARGS);
  extern Datum numeric_text(PG_FUNCTION_ARGS);
  extern Datum numeric_accum(PG_FUNCTION_ARGS);
+ extern Datum numeric_avg_accum(PG_FUNCTION_ARGS);
  extern Datum int2_accum(PG_FUNCTION_ARGS);
  extern Datum int4_accum(PG_FUNCTION_ARGS);
  extern Datum int8_accum(PG_FUNCTION_ARGS);
+ extern Datum int8_avg_accum(PG_FUNCTION_ARGS);
  extern Datum numeric_avg(PG_FUNCTION_ARGS);
  extern Datum numeric_var_pop(PG_FUNCTION_ARGS);
  extern Datum numeric_var_samp(PG_FUNCTION_ARGS);

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] [PERFORM] Direct I/O issues
Next
From: Mark Kirkwood
Date:
Subject: Re: Avg performance for int8/numeric