Stephen Frost <[EMAIL PROTECTED]> writes: > * Neil Conway ([EMAIL PROTECTED]) wrote: >> 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(). Unless the function actually *needs* to be non-strict, you should mark it strict and omit the runtime test for null input altogether. This is the general way that it's done in existing backend C functions. Doing it the other way is needlessly inconsistent (thus distracting readers) and clutters the code. (However, now that we support nulls in arrays, meseems a more consistent definition would be that it allows null inputs and just includes them in the output. So probably you do need it non-strict.) Personally though I'm much more concerned about the state datatype. As-is I think it's not only ugly but probably a security hole. If you are declaring the state type as something other than what it really is then you have to defend against two sorts of problems: someone being able to crash the database by calling your function and passing it something it didn't expect, or crashing the database by using your function to pass some other function an input it didn't expect. For example, since you've got aaccum_sfunc declared to return anyarray when it returns no such thing, something like array_out(aaccum_sfunc(...)) would trivially crash the backend. It's possible that the error check to insist on being called with an AggState context is a sufficient defense against that, but I feel nervous about it, and would much rather have a solution that isn't playing fast and loose with the type system. Particularly if it's going to go into core rather than contrib. I'm inclined to think that this example demonstrates a deficiency in the aggregate-function design: there should be a way to declare what we're really doing. But I don't know exactly what that should look like. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq