Re: Patch: Add parse_type Function

Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> writes:

> 1) The function causes a crash when called with a NULL parameter:
>
> SELECT * FROM parse_type(NULL);
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
>
> You have to check it in the beginning of function and either return an
> error message or just NULL, e.g
>
>     if (PG_ARGISNULL(0))
>         PG_RETURN_NULL();

Instead of handling this in the function body, the function should be
declared as strict. This is in fact the default in pg_proc.h,

/* strict with respect to NULLs? */
bool proisstrict BKI_DEFAULT(t);

so removing the explicit proisstrict => 'f' from the pg_proc.dat entry
will fix it with no additional code.

> 2) Add a function call with NULL in the regression tests
>
> SELECT * FROM parse_type(NULL);
>
> 3) Add the expected behaviour of calling the function with NULL in the
> docs (error message or null)

Once the function is declared strict, I don't think either of these is
necessary: function strictness is tested elsewhere, and it's the default
behaviour. The only functions that explicitly say they return NULL on
NULL inputs are quote_literal (because you might expect it to return the
string 'NULL', but there's qoute_nullable for that) and xmlexists (which
I don't see any particular reason for).

- ilmari