Re: PostgreSQL 8.3.4 reproducible crash

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: dmitry(at)koterov(dot)ru
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: PostgreSQL 8.3.4 reproducible crash
Date: 2008-12-10 13:16:16
Message-ID: 493FC120.7010707@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 work to 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2008-12-10 13:26:57 Re: WIP: default values for function parameters
Previous Message Bruce Momjian 2008-12-10 13:00:01 Re: ALTER composite type does not work, but ALTER TABLE which ROWTYPE is used as a type - works fine