Re: PostgreSQL 8.3.4 reproducible crash

Dmitry Koterov wrote:
> Hello.
> 
> Here is the SQL to reproduce the server crash:
> 
> 
> CREATE SCHEMA bug1 AUTHORIZATION postgres;
> 
> SET search_path = bug1, pg_catalog;
> 
> CREATE FUNCTION bug1.domain_check (integer) RETURNS boolean
> AS
> $body$
> SELECT $1 <> 0
> $body$
> LANGUAGE sql IMMUTABLE STRICT;
> 
> CREATE DOMAIN bug1."domain" AS integer
>         CONSTRAINT "check" CHECK (bug1.domain_check(VALUE));
> 
> CREATE TYPE bug1.composite AS (
>   id domain
> );
> 
> select '(1)'::bug1.composite;

Reproducible on 8.2 as well.

It's crashing in execQual.c, on line 299 (on REL_8_3_STABLE):/* * In a read-only function, use the surrounding query's
snapshot;* otherwise take a new snapshot for each query.  The snapshot should * include a fresh command ID so that all
workto date in this transaction * is visible.    We copy in both cases so that postquel_end can * unconditionally do
FreeSnapshot.*/if (fcache->readonly_func)>>>        snapshot = CopySnapshot(ActiveSnapshot);else{
CommandCounterIncrement();   snapshot = CopySnapshot(GetTransactionSnapshot());}
 

because ActiveSnapshot is NULL. ActiveSnapshot has not yet been set, 
because the input function and domain checking is done in the parse 
analyze phase.

I propose that we simply add add a NULL-check there:

*** src/backend/executor/functions.c    1 Jan 2008 19:45:49 -0000 
1.120
--- src/backend/executor/functions.c    10 Dec 2008 13:13:25 -0000
***************
*** 295,301 ****         * is visible.  We copy in both cases so that postquel_end can         * unconditionally do
FreeSnapshot.        */
 
!       if (fcache->readonly_func)                snapshot = CopySnapshot(ActiveSnapshot);        else        {
--- 295,301 ----         * is visible.  We copy in both cases so that postquel_end can         * unconditionally do
FreeSnapshot.        */
 
!       if (fcache->readonly_func && ActiveSnapshot != NULL)                snapshot = CopySnapshot(ActiveSnapshot);
   else        {
 



8.1 and before are not vulnerable to this, but only because the domain 
check wasn't invoked at all in cases like this:

> postgres=# select '(0)'::bug1.composite;
>  composite 
> -----------
>  (0)
> (1 row)

That was a known issue that was fixed in 8.2.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com