To: "Rajesh Kumar Mallah" <mallah.rajesh@gmail.com>
cc: pgsql-sql@postgresql.org, pgsql-bugs@postgresql.org
Subject: Re: [SQL] (possible) bug with constraint exclusion 
In-reply-to: <a97c77030801102258y3d501045u6b31afe0541997db@mail.gmail.com> 
References: <a97c77030801102258y3d501045u6b31afe0541997db@mail.gmail.com>
Comments: In-reply-to "Rajesh Kumar Mallah" <mallah.rajesh@gmail.com>
	message dated "Fri, 11 Jan 2008 12:28:09 +0530"
Date: Fri, 11 Jan 2008 14:56:08 -0500
Message-ID: <23924.1200081368@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>

"Rajesh Kumar Mallah" <mallah.rajesh@gmail.com> writes:
> looks like constraint exclusion is being too aggressive in excluding null values

Hmm, you're right.  Looks like I broke it here:
http://archives.postgresql.org/pgsql-committers/2007-05/msg00187.php

> although its well known that check constraints apply on not null values only.

No, that is not a correct statement either --- it's exactly that type of
sloppy thinking that got me into trouble with this patch :-(

The problem is that predicate_refuted_by_simple_clause() is failing to
distinguish whether "refutes" means "proves false" or "proves not true".
For constraint exclusion we have to use the stricter "proves false"
interpretation, and in that scenario a clause "foo IS NULL" fails to
refute a check constraint "foo > 0", because the latter will produce
NULL which isn't false and therefore doesn't cause the check constraint
to fail.

The motivation for that patch was to support IS NULL as one partition
of a partitioned table.  Thinking about it I see that if the other
partitions have check constraints like "foo > 0" then the partitioning
is actually incorrect, because the other check constraints are failing
to exclude NULLs.  The right way to set up such a partitioned table is
to include "foo IS NOT NULL" as part of the check constraint, or as
a special-purpose NOT NULL flag, except in the IS NULL partition.
The current constraint exclusion logic fails to notice attnotnull,
though.  So the correct fix seems to be:

* Fix predicate_refuted_by_simple_clause to not suppose that a strict
operator is proved FALSE by an IS NULL clause.

* Fix relation_excluded_by_constraints to add "foo IS NOT NULL" clauses
to the constraint list for attnotnull columns (perhaps this should be
pushed into get_relation_constraints?).  This buys back the loss of
exclusion from the other change, so long as the partitioning is done
correctly.

			regards, tom lane