Fix improper uses of canonicalize_qual().

One of the things canonicalize_qual() does is to remove constant-NULL
subexpressions of top-level AND/OR clauses.  It does that on the assumption
that what it's given is a top-level WHERE clause, so that NULL can be
treated like FALSE.  Although this is documented down inside a subroutine
of canonicalize_qual(), it wasn't mentioned in the documentation of that
function itself, and some callers hadn't gotten that memo.

Notably, commit d007a9505 caused get_relation_constraints() to apply
canonicalize_qual() to CHECK constraints.  That allowed constraint
exclusion to misoptimize situations in which a CHECK constraint had a
provably-NULL subclause, as seen in the regression test case added here,
in which a child table that should be scanned is not.  (Although this
thinko is ancient, the test case doesn't fail before 9.2, for reasons
I've not bothered to track down in detail.  There may be related cases
that do fail before that.)

More recently, commit f0e44751d added an independent bug by applying
canonicalize_qual() to index expressions, which is even sillier since
those might not even be boolean.  If they are, though, I think this
could lead to making incorrect index entries for affected index
expressions in v10.  I haven't attempted to prove that though.

To fix, add an "is_check" parameter to canonicalize_qual() to specify
whether it should assume WHERE or CHECK semantics, and make it perform
NULL-elimination accordingly.  Adjust the callers to apply the right
semantics, or remove the call entirely in cases where it's not known
that the expression has one or the other semantics.  I also removed
the call in some cases involving partition expressions, where it should
be a no-op because such expressions should be canonical already ...
and was a no-op, independently of whether it could in principle have
done something, because it was being handed the qual in implicit-AND
format which isn't what it expects.  In HEAD, add an Assert to catch
that type of mistake in future.

This represents an API break for external callers of canonicalize_qual().
While that's intentional in HEAD to make such callers think about which
case applies to them, it seems like something we probably wouldn't be
thanked for in released branches.  Hence, in released branches, the
extra parameter is added to a new function canonicalize_qual_ext(),
and canonicalize_qual() is a wrapper that retains its old behavior.

Patch by me with suggestions from Dean Rasheed.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/24475.1520635069@sss.pgh.pa.us

src/test/modules/test_predtest/test_predtest.c

@@ -19,7 +19,6 @@
 #include "funcapi.h"
 #include "optimizer/clauses.h"
 #include "optimizer/predtest.h"
-#include "optimizer/prep.h"
 #include "utils/builtins.h"
 
 PG_MODULE_MAGIC;
@@ -137,18 +136,18 @@ test_predtest(PG_FUNCTION_ARGS)
 
 	/*
 	 * Because the clauses are in the SELECT list, preprocess_expression did
-	 * not pass them through canonicalize_qual nor make_ands_implicit.  We can
-	 * do that here, though, and should do so to match the planner's normal
-	 * usage of the predicate proof functions.
+	 * not pass them through canonicalize_qual nor make_ands_implicit.
 	 *
-	 * This still does not exactly duplicate the normal usage of the proof
-	 * functions, in that they are often given qual clauses containing
-	 * RestrictInfo nodes.  But since predtest.c just looks through those
-	 * anyway, it seems OK to not worry about that point.
+	 * We can't do canonicalize_qual here, since it's unclear whether the
+	 * expressions ought to be treated as WHERE or CHECK clauses. Fortunately,
+	 * useful test expressions wouldn't be affected by those transformations
+	 * anyway.  We should do make_ands_implicit, though.
+	 *
+	 * Another way in which this does not exactly duplicate the normal usage
+	 * of the proof functions is that they are often given qual clauses
+	 * containing RestrictInfo nodes.  But since predtest.c just looks through
+	 * those anyway, it seems OK to not worry about that point.
 	 */
-	clause1 = canonicalize_qual(clause1);
-	clause2 = canonicalize_qual(clause2);
-
 	clause1 = (Expr *) make_ands_implicit(clause1);
 	clause2 = (Expr *) make_ands_implicit(clause2);
 

src/test/regress/expected/inherit.out

@@ -1661,6 +1661,30 @@ reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
 --
+-- Check handling of a constant-null CHECK constraint
+--
+create table cnullparent (f1 int);
+create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent);
+insert into cnullchild values(1);
+insert into cnullchild values(2);
+insert into cnullchild values(null);
+select * from cnullparent;
+ f1 
+----
+  1
+  2
+   
+(3 rows)
+
+select * from cnullparent where f1 = 2;
+ f1 
+----
+  2
+(1 row)
+
+drop table cnullparent cascade;
+NOTICE:  drop cascades to table cnullchild
+--
 -- Check that constraint exclusion works correctly with partitions using
 -- implicit constraints generated from the partition bound information.
 --

src/test/regress/sql/inherit.sql

@@ -611,6 +611,18 @@ reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
 
+--
+-- Check handling of a constant-null CHECK constraint
+--
+create table cnullparent (f1 int);
+create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent);
+insert into cnullchild values(1);
+insert into cnullchild values(2);
+insert into cnullchild values(null);
+select * from cnullparent;
+select * from cnullparent where f1 = 2;
+drop table cnullparent cascade;
+
 --
 -- Check that constraint exclusion works correctly with partitions using
 -- implicit constraints generated from the partition bound information.