Обсуждение: (possible) bug with constraint exclusion
Hi , looks like constraint exclusion is being too aggressive in excluding null values although its well known that check constraints apply on not null values only. Hope the minimal test session below explains the problem we facing. BTW: we are very impressed with the performance gains we achieved by partitioning a table recently. tradein_clients=> SELECT version(); version ---------------------------------------------------------------------------------------------------PostgreSQL 8.2.6 on i686-pc-linux-gnu,compiled by GCC gcc (GCC) 3.4.6 20060404 (Red Hat 3.4.6-9) (1 row) tradein_clients=> \pset null NULL tradein_clients=> \d x Table "temp.x"Column | Type | Modifiers --------+---------+-----------id | integer | Check constraints: "x_id_check" CHECK (id > 0) tradein_clients=> SELECT * from x; id ------ 1 2NULL (3 rows) tradein_clients=> explain SELECT * from x where id is null; QUERY PLAN ------------------------------------------Result (cost=0.00..0.01 rows=1 width=0) One-Time Filter: false (2 rows) tradein_clients=> SELECT * from x where id is null;id ---- (0 rows) tradein_clients=> SET constraint_exclusion to off; SET tradein_clients=> SELECT * from x where id is null; id ------NULL (1 row) tradein_clients=> Regds mallah.
Update the phenomenon does not exists in 8.2.0 but exists in 8.2.5. On Jan 11, 2008 12:28 PM, Rajesh Kumar Mallah <mallah.rajesh@gmail.com> wrote: > Hi , > > looks like constraint exclusion is being too aggressive in excluding null values > although its well known that check constraints apply on not null values only. > Hope the minimal test session below explains the problem we facing. > BTW: we are very impressed with the performance gains we achieved by > partitioning a table recently. > > > > tradein_clients=> SELECT version(); > version > --------------------------------------------------------------------------------------------------- > PostgreSQL 8.2.6 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) > 3.4.6 20060404 (Red Hat 3.4.6-9) > (1 row) > > tradein_clients=> \pset null NULL > > > tradein_clients=> \d x > Table "temp.x" > Column | Type | Modifiers > --------+---------+----------- > id | integer | > Check constraints: > "x_id_check" CHECK (id > 0) > > tradein_clients=> SELECT * from x; > id > ------ > 1 > 2 > NULL > (3 rows) > > tradein_clients=> explain SELECT * from x where id is null; > QUERY PLAN > ------------------------------------------ > Result (cost=0.00..0.01 rows=1 width=0) > One-Time Filter: false > (2 rows) > > tradein_clients=> SELECT * from x where id is null; > id > ---- > (0 rows) > tradein_clients=> SET constraint_exclusion to off; > SET > tradein_clients=> SELECT * from x where id is null; > id > ------ > NULL > (1 row) > > tradein_clients=> > > Regds > mallah. >
"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
On Jan 12, 2008 1:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "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: Dear Tom, Thanks for the elaborate explanation on your part, owing to my limitations I could not understand all the parts of it. Am I correct in understanding that the current behavior is inappropriate and shall be corrected at some point of time in future versions ? thanks once again to all the developers for making PostgreSQL. regds mallah. > > * 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 >
"Rajesh Kumar Mallah" <mallah.rajesh@gmail.com> writes: > Am I correct in understanding that the current behavior is inappropriate > and shall be corrected at some point of time in future versions ? It's a bug, it's patched: http://archives.postgresql.org/pgsql-committers/2008-01/msg00184.php regards, tom lane
On Jan 12, 2008 10:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > "Rajesh Kumar Mallah" <mallah.rajesh@gmail.com> writes: > > Am I correct in understanding that the current behavior is inappropriate > > and shall be corrected at some point of time in future versions ? > > It's a bug, it's patched: > http://archives.postgresql.org/pgsql-committers/2008-01/msg00184.php Thanks for the (unbelievable) quick response, I applied the patch on my production server and the problem is gone. tradein_clients=> \pset null NULL Null display is "NULL". tradein_clients=> SELECT * from temp.x where id is NULL; id ------NULL (1 row) tradein_clients=> SELECT * from temp.x ; id ------ 1 2NULL (3 rows) tradein_clients=> Regds mallah. > > regards, tom lane >
Hi list, > It's a bug, it's patched: > http://archives.postgresql.org/pgsql-committers/2008-01/msg00184.php > I have just stumbled on the same bug today and was very happy to find a patch; however, I have a 8.2.6 server running which of course cannot be patched. (According to the CVS tags the revisions of plancat.c and predtest.c in the 8.2.6 release are 1.127 and 1.10.2.2, respectively, whereas the patch is against the head revisions.) Would it be possible to provide a patch against the 8.2.6 stable release (the one that can be downloaded from the download section of the homepage)? It seems as if partitioning is unusable without this patch because when constraint exclusion is enabled, records cannot be found by select or update even in tables where partitioning is not realized at all. On the other hand, without constraint exclusion, partitioning at all doesn't make sense (as far as I understand). Thanks a lot for your help! Regards, Christian -- Deriva GmbH Tel.: +49 551 489500-42 Financial IT and Consulting Fax: +49 551 489500-91 Hans-Böckler-Straße 2 http://www.deriva.de D-37079 Göttingen Deriva CA Certificate: http://www.deriva.de/deriva-ca.cer
Christian Schröder <cs@deriva.de> writes: >> It's a bug, it's patched: >> http://archives.postgresql.org/pgsql-committers/2008-01/msg00184.php >> > I have just stumbled on the same bug today and was very happy to find a > patch; however, I have a 8.2.6 server running which of course cannot be > patched. (According to the CVS tags the revisions of plancat.c and > predtest.c in the 8.2.6 release are 1.127 and 1.10.2.2, respectively, > whereas the patch is against the head revisions.) Better look again. regards, tom lane
Tom Lane wrote: > Better look again. > Sounds like a sensible advice ... I somehow managed to find http://archives.postgresql.org/pgsql-committers/2008-01/msg00183.php instead of http://archives.postgresql.org/pgsql-committers/2008-01/msg00184.php ... Sorry for that! Regards, Christian -- Deriva GmbH Tel.: +49 551 489500-42 Financial IT and Consulting Fax: +49 551 489500-91 Hans-Böckler-Straße 2 http://www.deriva.de D-37079 Göttingen Deriva CA Certificate: http://www.deriva.de/deriva-ca.cer