Clean up the usage of canonicalize_qual(): in particular, be consistent

about whether it is applied before or after eval_const_expressions().
I believe there were some corner cases where the system would fail to
recognize that a partial index is applicable because of the previous
inconsistency.  Store normal rather than 'implicit AND' representations
of constraints and index predicates in the catalogs.
initdb forced due to representation change of constraints/predicates.

src/backend/catalog/heap.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.256 2003/11/29 19:51:42 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.257 2003/12/28 21:57:36 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1344,10 +1344,9 @@ StoreRelCheck(Relation rel, char *ccname, char *ccbin)
 	int16	   *attNos;
 
 	/*
-	 * Convert condition to a normal boolean expression tree.
+	 * Convert condition to an expression tree.
 	 */
 	expr = stringToNode(ccbin);
-	expr = (Node *) make_ands_explicit((List *) expr);
 
 	/*
 	 * deparse it
@@ -1651,14 +1650,6 @@ AddRelationRawConstraints(Relation rel,
 					(errcode(ERRCODE_GROUPING_ERROR),
 					 errmsg("cannot use aggregate function in check constraint")));
 
-		/*
-		 * Constraints are evaluated with execQual, which expects an
-		 * implicit-AND list, so convert expression to implicit-AND form.
-		 * (We could go so far as to convert to CNF, but that's probably
-		 * overkill...)
-		 */
-		expr = (Node *) make_ands_implicit((Expr *) expr);
-
 		/*
 		 * OK, store it.
 		 */

src/backend/catalog/index.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.223 2003/11/29 19:51:43 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.224 2003/12/28 21:57:36 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -397,13 +397,14 @@ UpdateIndexRelation(Oid indexoid,
 		exprsDatum = (Datum) 0;
 
 	/*
-	 * Convert the index predicate (if any) to a text datum
+	 * Convert the index predicate (if any) to a text datum.  Note we
+	 * convert implicit-AND format to normal explicit-AND for storage.
 	 */
 	if (indexInfo->ii_Predicate != NIL)
 	{
 		char	   *predString;
 
-		predString = nodeToString(indexInfo->ii_Predicate);
+		predString = nodeToString(make_ands_explicit(indexInfo->ii_Predicate));
 		predDatum = DirectFunctionCall1(textin,
 										CStringGetDatum(predString));
 		pfree(predString);

src/backend/commands/indexcmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.116 2003/11/29 19:51:47 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.117 2003/12/28 21:57:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,7 +42,7 @@
 
 
 /* non-export function prototypes */
-static void CheckPredicate(List *predList);
+static void CheckPredicate(Expr *predicate);
 static void ComputeIndexAttrs(IndexInfo *indexInfo, Oid *classOidP,
 				  List *attList,
 				  Oid relId,
@@ -80,7 +80,6 @@ DefineIndex(RangeVar *heapRelation,
 	Form_pg_am	accessMethodForm;
 	IndexInfo  *indexInfo;
 	int			numberOfAttributes;
-	List	   *cnfPred = NIL;
 
 	/*
 	 * count attributes in index
@@ -172,16 +171,10 @@ DefineIndex(RangeVar *heapRelation,
 	}
 
 	/*
-	 * Convert the partial-index predicate from parsetree form to an
-	 * implicit-AND qual expression, for easier evaluation at runtime.
-	 * While we are at it, we reduce it to a canonical (CNF or DNF) form
-	 * to simplify the task of proving implications.
+	 * Validate predicate, if given
 	 */
 	if (predicate)
-	{
-		cnfPred = canonicalize_qual((Expr *) copyObject(predicate), true);
-		CheckPredicate(cnfPred);
-	}
+		CheckPredicate(predicate);
 
 	/*
 	 * Check that all of the attributes in a primary key are marked as not
@@ -237,13 +230,13 @@ DefineIndex(RangeVar *heapRelation,
 
 	/*
 	 * Prepare arguments for index_create, primarily an IndexInfo
-	 * structure
+	 * structure.  Note that ii_Predicate must be in implicit-AND format.
 	 */
 	indexInfo = makeNode(IndexInfo);
 	indexInfo->ii_NumIndexAttrs = numberOfAttributes;
 	indexInfo->ii_Expressions = NIL;	/* for now */
 	indexInfo->ii_ExpressionsState = NIL;
-	indexInfo->ii_Predicate = cnfPred;
+	indexInfo->ii_Predicate = make_ands_implicit(predicate);
 	indexInfo->ii_PredicateState = NIL;
 	indexInfo->ii_Unique = unique;
 
@@ -268,7 +261,7 @@ DefineIndex(RangeVar *heapRelation,
 
 /*
  * CheckPredicate
- *		Checks that the given list of partial-index predicates is valid.
+ *		Checks that the given partial-index predicate is valid.
  *
  * This used to also constrain the form of the predicate to forms that
  * indxpath.c could do something with.	However, that seems overly
@@ -278,18 +271,18 @@ DefineIndex(RangeVar *heapRelation,
  * (except ones requiring a plan), and let indxpath.c fend for itself.
  */
 static void
-CheckPredicate(List *predList)
+CheckPredicate(Expr *predicate)
 {
 	/*
 	 * We don't currently support generation of an actual query plan for a
 	 * predicate, only simple scalar expressions; hence these
 	 * restrictions.
 	 */
-	if (contain_subplans((Node *) predList))
+	if (contain_subplans((Node *) predicate))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot use subquery in index predicate")));
-	if (contain_agg_clause((Node *) predList))
+	if (contain_agg_clause((Node *) predicate))
 		ereport(ERROR,
 				(errcode(ERRCODE_GROUPING_ERROR),
 				 errmsg("cannot use aggregate in index predicate")));
@@ -298,7 +291,7 @@ CheckPredicate(List *predList)
 	 * A predicate using mutable functions is probably wrong, for the same
 	 * reasons that we don't allow an index expression to use one.
 	 */
-	if (contain_mutable_functions((Node *) predList))
+	if (contain_mutable_functions((Node *) predicate))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
 		errmsg("functions in index predicate must be marked IMMUTABLE")));

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.223 2003/12/01 22:07:58 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.224 2003/12/28 21:57:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -40,6 +40,7 @@
 #include "executor/execdebug.h"
 #include "executor/execdefs.h"
 #include "miscadmin.h"
+#include "optimizer/clauses.h"
 #include "optimizer/var.h"
 #include "parser/parsetree.h"
 #include "utils/acl.h"
@@ -1658,7 +1659,8 @@ ExecRelCheck(ResultRelInfo *resultRelInfo,
 			(List **) palloc(ncheck * sizeof(List *));
 		for (i = 0; i < ncheck; i++)
 		{
-			qual = (List *) stringToNode(check[i].ccbin);
+			/* ExecQual wants implicit-AND form */
+			qual = make_ands_implicit(stringToNode(check[i].ccbin));
 			resultRelInfo->ri_ConstraintExprs[i] = (List *)
 				ExecPrepareExpr((Expr *) qual, estate);
 		}

src/backend/optimizer/plan/planner.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.162 2003/11/29 19:51:50 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.163 2003/12/28 21:57:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -377,23 +377,13 @@ preprocess_expression(Query *parse, Node *expr, int kind)
 		expr = flatten_join_alias_vars(parse, expr);
 
 	/*
-	 * Simplify constant expressions.
-	 *
-	 * Note that at this point quals have not yet been converted to
-	 * implicit-AND form, so we can apply eval_const_expressions directly.
-	 */
-	expr = eval_const_expressions(expr);
-
-	/*
-	 * If it's a qual or havingQual, canonicalize it, and convert it to
-	 * implicit-AND format.
-	 *
-	 * XXX Is there any value in re-applying eval_const_expressions after
-	 * canonicalize_qual?
+	 * If it's a qual or havingQual, canonicalize it.  It seems most useful
+	 * to do this before applying eval_const_expressions, since the latter
+	 * can optimize flattened AND/ORs better than unflattened ones.
 	 */
 	if (kind == EXPRKIND_QUAL)
 	{
-		expr = (Node *) canonicalize_qual((Expr *) expr, true);
+		expr = (Node *) canonicalize_qual((Expr *) expr);
 
 #ifdef OPTIMIZER_DEBUG
 		printf("After canonicalize_qual()\n");
@@ -401,6 +391,19 @@ preprocess_expression(Query *parse, Node *expr, int kind)
 #endif
 	}
 
+	/*
+	 * Simplify constant expressions.
+	 */
+	expr = eval_const_expressions(expr);
+
+	/*
+	 * If it's a qual or havingQual, convert it to implicit-AND format.
+	 * (We don't want to do this before eval_const_expressions, since the
+	 * latter would be unable to simplify a top-level AND correctly.)
+	 */
+	if (kind == EXPRKIND_QUAL)
+		expr = (Node *) make_ands_implicit((Expr *) expr);
+
 	/* Expand SubLinks to SubPlans */
 	if (parse->hasSubLinks)
 		expr = SS_process_sublinks(expr, (kind == EXPRKIND_QUAL));

src/backend/optimizer/util/clauses.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.156 2003/12/09 01:56:20 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.157 2003/12/28 21:57:37 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -261,8 +261,8 @@ make_and_qual(Node *qual1, Node *qual2)
 }
 
 /*
- * Sometimes (such as in the result of canonicalize_qual or the input of
- * ExecQual), we use lists of expression nodes with implicit AND semantics.
+ * Sometimes (such as in the input of ExecQual), we use lists of expression
+ * nodes with implicit AND semantics.
  *
  * These functions convert between an AND-semantics expression list and the
  * ordinary representation of a boolean expression.

src/backend/utils/adt/ruleutils.c

@@ -3,7 +3,7 @@
  *				back to source text
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.160 2003/11/29 19:51:59 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.161 2003/12/28 21:57:37 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -810,13 +810,6 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, int prettyFlags)
 			node = (Node *) stringToNode(predString);
 			pfree(predString);
 
-			/*
-			 * If top level is a List, assume it is an implicit-AND
-			 * structure, and convert to explicit AND.	This is needed for
-			 * partial index predicates.
-			 */
-			if (node && IsA(node, List))
-				node = (Node *) make_ands_explicit((List *) node);
 			/* Deparse */
 			str = deparse_expression_pretty(node, context, false, false,
 											prettyFlags, 0);
@@ -1060,14 +1053,6 @@ pg_get_constraintdef_worker(Oid constraintId, int prettyFlags)
 				conbin = DatumGetCString(DirectFunctionCall1(textout, val));
 				expr = stringToNode(conbin);
 
-				/*
-				 * If top level is a List, assume it is an implicit-AND
-				 * structure, and convert to explicit AND.	This is needed
-				 * for partial index predicates.
-				 */
-				if (expr && IsA(expr, List))
-					expr = (Node *) make_ands_explicit((List *) expr);
-
 				/* Set up deparsing context for Var nodes in constraint */
 				if (conForm->conrelid != InvalidOid)
 				{
@@ -1212,14 +1197,6 @@ pg_get_expr_worker(text *expr, Oid relid, char *relname, int prettyFlags)
 	/* Convert expression to node tree */
 	node = (Node *) stringToNode(exprstr);
 
-	/*
-	 * If top level is a List, assume it is an implicit-AND structure, and
-	 * convert to explicit AND.  This is needed for partial index
-	 * predicates.
-	 */
-	if (node && IsA(node, List))
-		node = (Node *) make_ands_explicit((List *) node);
-
 	/* Deparse */
 	context = deparse_context_for(relname, relid);
 	str = deparse_expression_pretty(node, context, false, false,

src/backend/utils/adt/selfuncs.c

@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.150 2003/12/07 04:14:10 joe Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.151 2003/12/28 21:57:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3902,12 +3902,12 @@ genericcostestimate(Query *root, RelOptInfo *rel,
 	 * If the index is partial, AND the index predicate with the
 	 * explicitly given indexquals to produce a more accurate idea of the
 	 * index restriction.  This may produce redundant clauses, which we
-	 * hope that cnfify and clauselist_selectivity will deal with
+	 * hope that canonicalize_qual and clauselist_selectivity will deal with
 	 * intelligently.
 	 *
 	 * Note that index->indpred and indexQuals are both in implicit-AND form
 	 * to start with, which we have to make explicit to hand to
-	 * canonicalize_qual, and then we get back implicit-AND form again.
+	 * canonicalize_qual, and then we convert back to implicit-AND form.
 	 */
 	if (index->indpred != NIL)
 	{
@@ -3915,7 +3915,8 @@ genericcostestimate(Query *root, RelOptInfo *rel,
 
 		andedQuals = make_ands_explicit(nconc(listCopy(index->indpred),
 											  indexQuals));
-		selectivityQuals = canonicalize_qual(andedQuals, true);
+		andedQuals = canonicalize_qual(andedQuals);
+		selectivityQuals = make_ands_implicit(andedQuals);
 	}
 
 	/* Estimate the fraction of main-table tuples that will be visited */

src/backend/utils/cache/relcache.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.193 2003/11/29 19:52:00 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.194 2003/12/28 21:57:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,7 @@
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
 #include "optimizer/planmain.h"
+#include "optimizer/prep.h"
 #include "storage/smgr.h"
 #include "utils/builtins.h"
 #include "utils/catcache.h"
@@ -2770,11 +2771,13 @@ RelationGetIndexExpressions(Relation relation)
 	pfree(exprsString);
 
 	/*
-	 * Run the expressions through eval_const_expressions.	This is not
-	 * just an optimization, but is necessary, because the planner will be
-	 * comparing them to const-folded qual clauses, and may fail to detect
-	 * valid matches without this.
+	 * Run the expressions through flatten_andors and eval_const_expressions.
+	 * This is not just an optimization, but is necessary, because the planner
+	 * will be comparing them to similarly-processed qual clauses, and may
+	 * fail to detect valid matches without this.
 	 */
+	result = (List *) flatten_andors((Node *) result);
+
 	result = (List *) eval_const_expressions((Node *) result);
 
 	/* May as well fix opfuncids too */
@@ -2791,7 +2794,8 @@ RelationGetIndexExpressions(Relation relation)
 /*
  * RelationGetIndexPredicate -- get the index predicate for an index
  *
- * We cache the result of transforming pg_index.indpred into a node tree.
+ * We cache the result of transforming pg_index.indpred into an implicit-AND
+ * node tree (suitable for ExecQual).
  * If the rel is not an index or has no predicate, we return NIL.
  * Otherwise, the returned tree is copied into the caller's memory context.
  * (We don't want to return a pointer to the relcache copy, since it could
@@ -2835,13 +2839,18 @@ RelationGetIndexPredicate(Relation relation)
 	pfree(predString);
 
 	/*
-	 * Run the expression through eval_const_expressions.  This is not
-	 * just an optimization, but is necessary, because the planner will be
-	 * comparing it to const-folded qual clauses, and may fail to detect
-	 * valid matches without this.
+	 * Run the expression through canonicalize_qual and eval_const_expressions.
+	 * This is not just an optimization, but is necessary, because the planner
+	 * will be comparing it to similarly-processed qual clauses, and may fail
+	 * to detect valid matches without this.
 	 */
+	result = (List *) canonicalize_qual((Expr *) result);
+
 	result = (List *) eval_const_expressions((Node *) result);
 
+	/* Also convert to implicit-AND format */
+	result = make_ands_implicit((Expr *) result);
+
 	/* May as well fix opfuncids too */
 	fix_opfuncids((Node *) result);
 

src/include/catalog/catversion.h

@@ -37,7 +37,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.213 2003/12/03 18:53:52 joe Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.214 2003/12/28 21:57:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	200312031
+#define CATALOG_VERSION_NO	200312281
 
 #endif

src/include/nodes/relation.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.86 2003/11/29 22:41:06 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.87 2003/12/28 21:57:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -245,7 +245,8 @@ typedef struct RelOptInfo
  *		zero entry, rather than looking at ncolumns.
  *
  *		The indexprs and indpred expressions have been run through
- *		eval_const_expressions() for ease of matching to WHERE clauses.
+ *		prepqual.c and eval_const_expressions() for ease of matching to
+ *		WHERE clauses.  indpred is in implicit-AND form.
  */
 
 typedef struct IndexOptInfo

src/include/optimizer/prep.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.42 2003/11/29 22:41:07 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.43 2003/12/28 21:57:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,8 +36,8 @@ extern Relids get_relids_for_join(Query *parse, int joinrelid);
 /*
  * prototypes for prepqual.c
  */
-extern List *canonicalize_qual(Expr *qual, bool removeAndFlag);
-extern List *cnfify(Expr *qual, bool removeAndFlag);
+extern Expr *canonicalize_qual(Expr *qual);
+extern Node *flatten_andors(Node *node);
 
 /*
  * prototypes for preptlist.c