Skip site navigation (1) Skip section navigation (2)

Re: array_accum aggregate

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org, Joe Conway <mail(at)joeconway(dot)com>
Subject: Re: array_accum aggregate
Date: 2006-10-12 17:31:33
Message-ID: 20061012173133.GG24675@kenobi.snowman.net (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
* Neil Conway (neilc(at)samurai(dot)com) wrote:
> On Wed, 2006-10-11 at 00:51 -0400, Stephen Frost wrote:
> >      Here, the actual state type for any aggregate call is the array type
> > !    having the actual input type as elements.  Note: array_accum() is now
> > !    a built-in aggregate which uses a much more efficient mechanism than
> > !    that which is provided by array_append, prior users of array_accum()
> > !    may be pleasantly suprised at the marked improvment for larger arrays.
> >     </para>
> 
> Not worth documenting, I think.

Ok.  Either way is fine with me, honestly.

> > *** 15,20 ****
> > --- 15,22 ----
> >   #include "utils/array.h"
> >   #include "utils/builtins.h"
> >   #include "utils/lsyscache.h"
> > + #include "utils/memutils.h"
> > + #include "nodes/execnodes.h"
> 
> Include directives should be sorted roughly alphabetically, with the
> exception of postgres.h and system headers.

Ah, yeah, you know I noticed that when I added memutils but wasn't
thinking when I went back and added execnodes (I had been trying to
minimize the number of includes by adding ones I needed one at a time
and seeing if any more were necessary).

> > + /* Structure, used by aaccum_sfunc and aaccum_ffunc to
> > +  * implement the array_accum() aggregate, for storing 
> > +  * pointers to the ArrayBuildState for the array we are 
> > +  * building and the MemoryContext in which it is being
> > +  * built.  Note that this structure is 
> > +  * considered an 'anyarray' externally, which is a
> > +  * variable-length datatype, and therefore
> > +  * must open with an int32 defining the length. */
> 
> Gross.

Indeed. :/

> > + 	/* Make sure we are being called in an aggregate. */
> > + 	if (!fcinfo->context || !IsA(fcinfo->context, AggState))
> > + 		ereport(ERROR,
> > + 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > + 				 errmsg("Can not call aaccum_sfunc as a non-aggregate"),
> > + 				 errhint("Use the array_accum aggregate")));
> 
> Per the error message style guidelines, errmsg() should begin with a
> lowercase letter and errhint() should be a complete sentence (so it
> needs punctuation, for example).

Ah, ok, guess I should go read those.

> > + Datum
> > + aaccum_ffunc(PG_FUNCTION_ARGS)
> > + {
> > + 	aaccum_info		*ainfo;
> > + 
> > + 	/* Check if we are passed in a NULL */
> > + 	if (PG_ARGISNULL(0)) PG_RETURN_ARRAYTYPE_P(NULL);
> 
> There is no guarantee why SQL NULL and PG_RETURN_XYZ(NULL) refer to the
> same thing -- use PG_RETURN_NULL() to return a SQL NULL value, or just
> make the function strict.

Huh, alright.  I'll probably just change it to PG_RETURN_NULL().

	Thanks!

		Stephen

In response to

Responses

pgsql-hackers by date

Next:From: Tim TassonisDate: 2006-10-12 17:40:42
Subject: Re: more anti-postgresql FUD
Previous:From: Neil ConwayDate: 2006-10-12 17:22:17
Subject: Re: array_accum aggregate

pgsql-patches by date

Next:From: Tom LaneDate: 2006-10-12 18:02:22
Subject: Re: array_accum aggregate
Previous:From: Neil ConwayDate: 2006-10-12 17:22:17
Subject: Re: array_accum aggregate

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group